archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Polish removal of conduit shield, including legacy stripping for phabricator on phabricator oauth scenarios

Browse Source 
Summary: ...just in case that stuff happens in the "wild". also cleaned up the logic here since we no longer have the conduit conditionality.

Test Plan: made sure I didn't break JS on the site. reasoned about logic of my function and asking people PHP typing questions in job interviews.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T891

Differential Revision: https://secure.phabricator.com/D3269
This commit is contained in:
Bob Trahan
2012-08-13 16:05:56 -07:00
parent 8b8549128f
commit 7bb3c39cde
4 changed files with 8 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/aphront/response/AphrontAjaxResponse.php
View File

@@ -67,7 +67,7 @@ final class AphrontAjaxResponse extends AphrontResponse {
$this->error); $this->error);
$response_json = $this->encodeJSONForHTTPResponse($object); $response_json = $this->encodeJSONForHTTPResponse($object);
return $this->addJSONShield($response_json, $use_javelin_shield = true); return $this->addJSONShield($response_json);
} }
public function getHeaders() { public function getHeaders() {

2
src/aphront/response/AphrontJSONResponse.php
View File

@@ -44,7 +44,7 @@ final class AphrontJSONResponse extends AphrontResponse {
public function buildResponseString() { public function buildResponseString() {
$response = $this->encodeJSONForHTTPResponse($this->content); $response = $this->encodeJSONForHTTPResponse($this->content);
if ($this->shouldAddJSONShield()) { if ($this->shouldAddJSONShield()) {
$response = $this->addJSONShield($response, $use_javelin_shield = false); $response = $this->addJSONShield($response);
} }
return $response; return $response;
} }

8
src/aphront/response/AphrontResponse.php
View File

@@ -85,7 +85,7 @@ abstract class AphrontResponse {
return $response; return $response;
} }
protected function addJSONShield($json_response, $use_javelin_shield) { protected function addJSONShield($json_response) {
// Add a shield to prevent "JSON Hijacking" attacks where an attacker // Add a shield to prevent "JSON Hijacking" attacks where an attacker
// requests a JSON response using a normal <script /> tag and then uses // requests a JSON response using a normal <script /> tag and then uses
@@ -93,11 +93,7 @@ abstract class AphrontResponse {
// This header causes the browser to loop infinitely instead of handing over // This header causes the browser to loop infinitely instead of handing over
// sensitive data. // sensitive data.
// TODO: This is massively stupid: Javelin and Conduit use different $shield = 'for (;;);';
// shields.
$shield = $use_javelin_shield
? 'for (;;);'
: 'for(;;);';
$response = $shield.$json_response; $response = $shield.$json_response;

4
src/applications/auth/oauth/provider/PhabricatorOAuthProviderPhabricator.php
View File

@@ -104,6 +104,10 @@ extends PhabricatorOAuthProvider {
} }
public function setUserData($data) { public function setUserData($data) {
// legacy conditionally strip shield. see D3265 for discussion.
if (strpos($data, 'for(;;);') === 0) {
$data = substr($data, 8);
}
$data = idx(json_decode($data, true), 'result'); $data = idx(json_decode($data, true), 'result');
$this->validateUserData($data); $this->validateUserData($data);
$this->userData = $data; $this->userData = $data;