Skip to content

Commit

Permalink
Fix html decoding in Job Viewer -> Edit Search
Browse files Browse the repository at this point in the history 
Original Bug Report:

https://app.asana.com/0/342819846538629/828830620362347

When entering a Search Title that contains html encodable values ( <, >,
& etc ), saving and then editing the search causes the encoded title to
be editable, not the decoded title.
Example:
  - Search Title: Pwned!<script>alert(1)</script>
    - When viewed this displays correctly as text. But if you 'edit' the
search you will instead see
      the value below.
  - Encoded: Pwned!&lt;script&gt;alert(1)&lt;/script&gt;
    - This is what you see when editing the search.
   - If you save the search after editing it then it will then become:
     - Pwned!&amp;lt;script&amp;&lt/script&amp;&gt;

Description:

the .text element contains html content. but you have to put normal
text in the form field.
  • Loading branch information
@jpwhite4
jpwhite4 committed Mar 19, 2020
1 parent 870dcb8 commit 601e8d8
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 16 deletions.
48 changes: 34 additions & 14 deletions classes/Rest/Controllers/WarehouseControllerProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,38 @@ public function getHistoryByTitle(Request $request, Application $app, $realm, $t
throw new NotFoundException("", 404);
}

/**
* retrieve and sanitize the search history parameters for a request
* throws and exception if the parameters are missing.
* @param Request $request The request.
* @return array decoded search parameters.
* @throws MissingMandatoryParametersException If the required parameters are absent.
*/
private function getSearchParams(Request $request)
{
$data = $request->get('data');

if (!isset($data)) {
throw new MissingMandatoryParametersException(
'Malformed request. Expected \'data\' to be present.',
400
);
}

$decoded = json_decode($data, true);

if ($decoded === null || !isset($decoded['text']) ) {
throw new MissingMandatoryParametersException(
'Malformed request. Expected \'data.text\' to be present.',
400
);
}

$decoded['text'] = htmlspecialchars($decoded['text'], ENT_COMPAT | ENT_HTML5);

return $decoded;
}

/**
* Attempt to create a new Search History record with the provided 'data'
* form parameter.
Expand All @@ -474,13 +506,8 @@ public function createHistory(Request $request, Application $app)

$history = $this->getUserStore($user, $realm);

$data = $request->get('data');
$decoded = $this->getSearchParams($request);

if (!isset($data)) {
throw new MissingMandatoryParametersException('Malformed request. Expected \'data\' to be present.', 400);
}

$decoded = json_decode($data, true);
$recordId = $this->getIntParam($request, 'recordid');

$created = is_numeric($recordId)
Expand Down Expand Up @@ -527,20 +554,13 @@ public function updateHistory(Request $request, Application $app, $id)
$user = $this->authorize($request);

$action = 'updateHistory';
$required = array('data');


$params = $this->parseRestArguments($request, $required);
if (!isset($params) || !isset($params['data'])) {
throw new MissingMandatoryParametersException('Malformed request. Expected \'data\' to be present.', 400);
}
$data = $this->getSearchParams($request);

$realm = $this->getStringParam($request, 'realm', true);

$history = $this->getUserStore($user, $realm);

$data = json_decode($params['data'], true);

$result = $history->upsert($id, $data);

if (!isset($result['dtype'])) {
Expand Down
4 changes: 2 additions & 2 deletions html/gui/js/modules/job_viewer/SearchPanel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ XDMoD.Module.JobViewer.SearchPanel = Ext.extend(Ext.Panel, {
this._retrieveSelected(node);

this.ownerCt.setTitle("Editing Search: " + this.title);
Ext.getCmp('job-viewer-search-name').setValue(this.title);
Ext.getCmp('job-viewer-search-name').setValue(Ext.util.Format.htmlDecode(params.text));

if (!this.ownerCt.isVisible()) {
this.ownerCt.show();
Expand Down Expand Up @@ -1456,7 +1456,7 @@ XDMoD.Module.JobViewer.SearchPanel = Ext.extend(Ext.Panel, {
var selected = self._resultsToJSON(self._getSelectedRecords());

var params = {
text: Ext.util.Format.htmlEncode(title),
text: title,
searchterms: {
params: self.resultsStore.searchParams
},
Expand Down
53 changes: 53 additions & 0 deletions tests/integration/lib/Rest/JobViewerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,59 @@ public function testAdvancedSearchNoParams() {
$this->xdmodhelper->logout();
}

/**
* Test that the search history save new search correctly sanitizes the text input.
* @return void
*/
public function testSearchSaving() {

$input = array(
"text" => "Typing in <script>alert(1)</script>",
"searchterms" => array(
"params" => array(
"start_date" => "2020-01-09",
"end_date" => "2020-01-15",
"realm" => "Jobs",
"limit" => 24,
"start" => 0,
"params" => "{\"resource\":[\"1\"]}"
)
),
"results" => array(
array(
'resource' => 'pozidriv',
'name' => 'blah',
"jobid" => 42520494,
"text" => "pozidriv-5138",
"dtype" => "jobid",
"local_job_id" => "5138"
)
)
);

$this->xdmodhelper->authenticate('cd');
$response = $this->xdmodhelper->post(self::ENDPOINT . 'search/history', array('realm' => 'Jobs'), array('data' => json_encode($input)));

$this->assertEquals(200, $response[1]['http_code']);
$result = $response[0];
$this->assertTrue($result['success']);

$this->assertEquals($input['searchterms'], $result['results']['searchterms']);
$this->assertEquals($input['results'], $result['results']['results']);

$this->assertEquals('Typing in &lt;script&gt;alert(1)&lt;/script&gt;', $result['results']['text']);

// get the record id
$recordId = $result['results'][$result['results']['dtype']];

// remove the dummy saved search.
$response = $this->xdmodhelper->delete(self::ENDPOINT . 'search/history/' . $recordId, array('realm' => 'Jobs'));

$this->assertEquals(200, $response[1]['http_code']);
$result = $response[0];
$this->assertTrue($result['success']);
}

public function testJobMetadata() {
//TODO: Needs further integration for other realms.
if (!in_array("jobs", self::$XDMOD_REALMS)) {
Expand Down

0 comments on commit 601e8d8

Please sign in to comment.