Page MenuHomePhabricator

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

Authored by epriestley on Dec 13 2019, 6:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 28, 12:34 AM
Unknown Object (File)
Tue, Mar 26, 1:24 AM
Unknown Object (File)
Mon, Mar 25, 4:33 AM
Unknown Object (File)
Thu, Feb 29, 2:42 PM
Unknown Object (File)
Thu, Feb 29, 2:42 PM
Unknown Object (File)
Thu, Feb 29, 2:42 PM
Unknown Object (File)
Feb 20 2024, 9:57 AM
Unknown Object (File)
Feb 16 2024, 4:49 AM
Subscribers
None

Details

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` ]]

Diff Detail

Repository
rP Phabricator
Branch
xss1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 23713
Build 32605: Run Core Tests
Build 32604: arc lint + arc unit

Event Timeline

This revision was not accepted when it landed; it landed in state Needs Review.Dec 13 2019, 6:37 PM
epriestley requested review of this revision.
This revision was automatically updated to reflect the committed changes.