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.

Fix an XSS issue with certain high-priority remarkup rules embedded inside lower-priority link rules

Summary:
See <https://hackerone.com/reports/758002>. The link rules don't test that their parameters are flat text before using them in unsafe contexts.

Since almost all rules are lower-priority than these link rules, this behavior isn't obvious. However, two rules have broadly higher priority (monospaced text, and one variation of link rules has higher priority than the other), and the latter can be used to perform an XSS attack with input in the general form `()[ [[ ... | ... ]] ]` so that the inner link rule is evaluated first, then the outer link rule uses non-flat text in an unsafe way.

Test Plan:
Tested examples in HackerOne report. A simple example of broken (but not unsafe) behavior is:

```
[[ `x` | `y` ]]
```

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

epriestley 54bcbdab 4cd333b3

+24 -6
+16 -6
src/applications/phriction/markup/PhrictionRemarkupRule.php
··· 16 16 } 17 17 18 18 public function markupDocumentLink(array $matches) { 19 + $name = trim(idx($matches, 2, '')); 20 + if (empty($matches[2])) { 21 + $name = null; 22 + } 23 + 24 + $path = trim($matches[1]); 25 + 26 + if (!$this->isFlatText($name)) { 27 + return $matches[0]; 28 + } 29 + 30 + if (!$this->isFlatText($path)) { 31 + return $matches[0]; 32 + } 33 + 19 34 // If the link contains an anchor, separate that off first. 20 - $parts = explode('#', trim($matches[1]), 2); 35 + $parts = explode('#', $path, 2); 21 36 if (count($parts) == 2) { 22 37 $link = $parts[0]; 23 38 $anchor = $parts[1]; ··· 46 61 } 47 62 $link = implode('/', $base_parts).'/'; 48 63 } 49 - } 50 - 51 - $name = trim(idx($matches, 2, '')); 52 - if (empty($matches[2])) { 53 - $name = null; 54 64 } 55 65 56 66 // Link is now used for slug detection, so append a slash if one
+8
src/infrastructure/markup/markuprule/PhutilRemarkupDocumentLinkRule.php
··· 136 136 $uri = trim($matches[1]); 137 137 $name = trim(idx($matches, 2)); 138 138 139 + if (!$this->isFlatText($uri)) { 140 + return $matches[0]; 141 + } 142 + 143 + if (!$this->isFlatText($name)) { 144 + return $matches[0]; 145 + } 146 + 139 147 // If whatever is being linked to begins with "/" or "#", or has "://", 140 148 // or is "mailto:" or "tel:", treat it as a URI instead of a wiki page. 141 149 $is_uri = preg_match('@(^/)|(://)|(^#)|(^(?:mailto|tel):)@', $uri);