recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Further mitigate BREACH by reducing reflectiveness

Summary:
Ref T3684. The URI itself is reflected in a few places. It is generally not dangerous because we only let you add random stuff to the end of it for one or two controllers (e.g., the file download controller lets you add "/whatever.jpg"), but:

- Remove it entirely in the main request, since it serves no purpose.
- Remove query parameters in Ajax requests. These are available in DarkConsole proper.

Also mask a few things in the "Request" tab; I've never used these fields when debugging or during support, and they leak quasi-sensitive information that could get screenshotted or over-the-shoulder'd.

I didn't mitgate `__metablock__` because I think the threat is so close to 0 that it's not worthwhile.

Test Plan: Used Darkconsole, examined Requests tab.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3684

Differential Revision: https://secure.phabricator.com/D6699

epriestley d3e700ce 7298589c

+27 -6
+15 -4
src/aphront/console/plugin/DarkConsoleRequestPlugin.php
··· 38 38 39 39 $sections = array_merge($sections, $data); 40 40 41 + $mask = array( 42 + 'HTTP_COOKIE' => true, 43 + 'HTTP_X_PHABRICATOR_CSRF' => true, 44 + ); 45 + 41 46 $out = array(); 42 47 foreach ($sections as $header => $map) { 43 48 $rows = array(); 44 49 foreach ($map as $key => $value) { 45 - $rows[] = array( 46 - $key, 47 - (is_array($value) ? json_encode($value) : $value), 48 - ); 50 + if (isset($mask[$key])) { 51 + $rows[] = array( 52 + $key, 53 + phutil_tag('em', array(), '(Masked)')); 54 + } else { 55 + $rows[] = array( 56 + $key, 57 + (is_array($value) ? json_encode($value) : $value), 58 + ); 59 + } 49 60 } 50 61 51 62 $table = new AphrontTableView($rows);
+8 -1
src/aphront/response/AphrontAjaxResponse.php
··· 37 37 public function buildResponseString() { 38 38 $console = $this->getConsole(); 39 39 if ($console) { 40 + // NOTE: We're stripping query parameters here both for readability and 41 + // to mitigate BREACH and similar attacks. The parameters are available 42 + // in the "Request" tab, so this should not impact usability. See T3684. 43 + $uri = $this->getRequest()->getRequestURI(); 44 + $uri = new PhutilURI($uri); 45 + $uri->setQueryParams(array()); 46 + 40 47 Javelin::initBehavior( 41 48 'dark-console', 42 49 array( 43 - 'uri' => (string)$this->getRequest()->getRequestURI(), 50 + 'uri' => (string)$uri, 44 51 'key' => $console->getKey($this->getRequest()), 45 52 'color' => $console->getColor(), 46 53 ));
+4 -1
src/view/page/PhabricatorStandardPageView.php
··· 199 199 Javelin::initBehavior( 200 200 'dark-console', 201 201 array( 202 - 'uri' => $request ? (string)$request->getRequestURI() : '?', 202 + // NOTE: We use a generic label here to prevent input reflection 203 + // and mitigate compression attacks like BREACH. See discussion in 204 + // T3684. 205 + 'uri' => pht('Main Request'), 203 206 'selected' => $user ? $user->getConsoleTab() : null, 204 207 'visible' => $user ? (int)$user->getConsoleVisible() : true, 205 208 'headers' => $headers,