Page MenuHomePhabricator

Whitelist allowed editor protocols
ClosedPublic

Authored by epriestley on Mar 17 2014, 6:55 PM.
Tags
None
Referenced Files
F13167097: D8551.diff
Tue, May 7, 6:56 AM
Unknown Object (File)
Sun, May 5, 10:46 AM
Unknown Object (File)
Fri, May 3, 5:40 AM
Unknown Object (File)
Tue, Apr 30, 12:11 AM
Unknown Object (File)
Tue, Apr 30, 12:11 AM
Unknown Object (File)
Tue, Apr 30, 12:11 AM
Unknown Object (File)
Tue, Apr 30, 12:11 AM
Unknown Object (File)
Tue, Apr 30, 12:10 AM
Subscribers

Details

Reviewers
btrahan
Commits
Restricted Diffusion Commit
rP039b8e43b98c: Whitelist allowed editor protocols
Summary

This is the other half of D8548. Specifically, the attack here was to set your own editor link to javascript\n:... and then you could XSS yourself. This isn't a hugely damaging attack, but we can be more certain by adding a whitelist here.

We already whitelist linkable protocols in remarkup (uri.allowed-protocols) in general.

Test Plan

Tried to set and use valid/invalid editor URIs.

Screen_Shot_2014-03-17_at_11.45.33_AM.png (996×1 px, 115 KB)

Screen_Shot_2014-03-17_at_11.45.29_AM.png (996×1 px, 205 KB)

Diff Detail

Repository
rP Phabricator
Branch
whitelisteditorproto
Lint
Lint Passed
Unit
Tests Passed

Event Timeline

epriestley retitled this revision from to Whitelist allowed editor protocols.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
  • Do a cleaner job when no editor link is configured.
btrahan edited edge metadata.
btrahan added inline comments.
src/applications/config/option/PhabricatorSecurityConfigOptions.php
161

maybe link "Let us know" to the feedback article?

This revision is now accepted and ready to land.Mar 17 2014, 7:40 PM
epriestley edited edge metadata.
  • Link to documentation.
epriestley updated this revision to Diff 20296.

Closed by commit rP039b8e43b98c (authored by @epriestley).