SecurityTag
ActivePublic

Details

Description

Tasks related to enhancing the security of Phabricator.

Recent Activity

Tue, Jun 6

jasonrumney added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

It doesn't try to run evil.exe until I edit the cell (the csv version, and the modified xlsx I saved myself try to start on opening).

Tue, Jun 6, 3:45 AM · Security
epriestley added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

Here's a version with =cmd|'/C evil.exe'!A0 if that produces different results:

Tue, Jun 6, 3:35 AM · Security
jasonrumney added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

Actually, Excel 2016 is vulnerable. The test case is incomplete. The cell needs to be =cmd|'/C evil.exe'!A0 to trigger the issue.
By modifying the value in both the xlsx and csv files, I was able to blindly click yes on lots of warning prompts and start calc.exe. But I am not sure that modifying the value in the xlsx file is giving the same result as the export would (something may have been stripped off in the meantime...)

Tue, Jun 6, 3:33 AM · Security
jasonrumney added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

It seems an up to date patched Excel 2016 is not vulnerable.

Tue, Jun 6, 3:10 AM · Security
jasonrumney added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

Saving this file as csv also doesn't trigger the bug in Excel 2016 - on reopening, Excel misdetects the format as SYLK, warns about the extension not matching, then warns that an error occured during loading, and finally when you agree to continue loading as "a different format", it opens without running calc.exe or prompting to update cells.

Tue, Jun 6, 3:04 AM · Security

Mon, Jun 5

epriestley added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

Good to hear, thanks!

Mon, Jun 5, 10:16 PM · Security
cspeckmim added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

Checked on Excel 2016 and it didn't run calc.exe or prompt for "update cells". Also checked on Excel for mac~

Mon, Jun 5, 9:27 PM · Security
epriestley added a comment to T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.

If anyone actually has bona fide Excel.exe installed, you could try opening this file to double check that we're not currently vulnerable:

Mon, Jun 5, 8:35 PM · Security
epriestley created T12800: When Excel opens a CSV file, it just runs whatever arbitrary code might be in the file.
Mon, Jun 5, 8:31 PM · Security

May 18 2017

Herald updated subscribers of T9408: Upgrading: `dot` (Graphviz) support removed, changes to `figlet` and `cowsay`.
May 18 2017, 11:51 AM · Remarkup, Security, Installing & Upgrading

May 12 2017

epriestley triaged T12701: Harbormaster does not ignore additional whitespace in HTTP URLs, making invalid HTTP requests as Low priority.

T11632 is vaguely related.

May 12 2017, 5:49 PM · Security, Harbormaster, Bug Report

Apr 13 2017

chad closed T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for as Resolved by committing rP3d6049d0da5c: Remove CAN_JOIN policy from Conpherence.
Apr 13 2017, 4:19 PM · Conpherence (v4), Security
epriestley added a comment to T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for.

(And I think "Conpherence as an announcement tool" is a very marginal use case anyway -- today, Phame is probably better already?)

Apr 13 2017, 12:22 PM · Conpherence (v4), Security
epriestley added a comment to T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for.

That seems reasonable to me. We could implement a "read-only" equivalent later by having a flag like "require edit permission to send messages to this room" if we need it. That probably makes more sense anyway -- I can't really think of any use cases where a room is sort-of read-only, but the users who can post to it are different than users who can edit it.

Apr 13 2017, 12:20 PM · Conpherence (v4), Security
chad moved T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for from Backlog to v4 on the Conpherence board.
Apr 13 2017, 4:47 AM · Conpherence (v4), Security
chad added a revision to T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for: D17675: Remove CAN_JOIN policy from Conpherence.
Apr 13 2017, 4:11 AM · Conpherence (v4), Security
chad claimed T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for.
Apr 13 2017, 3:50 AM · Conpherence (v4), Security
chad added a comment to T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for.

I think we should remove the CAN_JOIN. If you can see it, you can join it. I don't see any parallel in Slack and looks like there are mods that add it forceably by kicking you off a channel if you leave a message. lulz.

Apr 13 2017, 3:49 AM · Conpherence (v4), Security

Apr 10 2017

epriestley added a comment to T12515: Upgrading: File Integrity Hashing and SHA1.

T12531: Unable to upload file: failed to read 4583864320 bytes after offset 0 discusses one side effect of these changes, it should be fixed in HEAD of master and stable now.

Apr 10 2017, 11:04 PM · Files, Guides, Security
epriestley closed T12526: parse_url() behavior has changed with PHP7, causing libphutil unit tests to fail and possibly creating security concerns as Resolved by committing rPHUfb9e0642c4ea: Reject ambiguous URIs with unescaped "#" or "?" in username/password parts.
Apr 10 2017, 5:34 PM · libphutil, Security
epriestley added a revision to T12526: parse_url() behavior has changed with PHP7, causing libphutil unit tests to fail and possibly creating security concerns: D17647: Reject ambiguous URIs with unescaped "#" or "?" in username/password parts.
Apr 10 2017, 4:54 PM · libphutil, Security
epriestley added a comment to T12526: parse_url() behavior has changed with PHP7, causing libphutil unit tests to fail and possibly creating security concerns.

I think this is, to at least some degree, a legitimate security problem. Consider:

Apr 10 2017, 4:26 PM · libphutil, Security
epriestley added a parent task for T12526: parse_url() behavior has changed with PHP7, causing libphutil unit tests to fail and possibly creating security concerns: T12101: Phabricator PHP 7 Compatibility.
Apr 10 2017, 3:46 PM · libphutil, Security
epriestley created T12526: parse_url() behavior has changed with PHP7, causing libphutil unit tests to fail and possibly creating security concerns.
Apr 10 2017, 1:26 PM · libphutil, Security

Apr 6 2017

epriestley created T12515: Upgrading: File Integrity Hashing and SHA1.
Apr 6 2017, 11:24 PM · Files, Guides, Security
epriestley added a commit to T12509: Plan the path forward from HMAC-SHA1: rP3d816e94dfbe: Rename "PhabricatorHash::digest()" to "weakDigest()".
Apr 6 2017, 10:43 PM · Infrastructure, Security
epriestley added a commit to T12509: Plan the path forward from HMAC-SHA1: rP3a3626834e60: Replace Remarkup calls to `PhabricatorHash::digest()` with SHA256.
Apr 6 2017, 10:43 PM · Infrastructure, Security
epriestley added a commit to T12509: Plan the path forward from HMAC-SHA1: rPd450a088906e: Support HMAC+SHA256 with automatic key generation and management.
Apr 6 2017, 10:43 PM · Infrastructure, Security
epriestley added a revision to T12509: Plan the path forward from HMAC-SHA1: D17632: Rename "PhabricatorHash::digest()" to "weakDigest()".
Apr 6 2017, 5:10 PM · Infrastructure, Security
epriestley added a revision to T12509: Plan the path forward from HMAC-SHA1: D17631: Replace Remarkup calls to `PhabricatorHash::digest()` with SHA256.
Apr 6 2017, 5:03 PM · Infrastructure, Security
epriestley added a revision to T12509: Plan the path forward from HMAC-SHA1: D17630: Support HMAC+SHA256 with automatic key generation and management.
Apr 6 2017, 4:38 PM · Infrastructure, Security
epriestley added a comment to T12509: Plan the path forward from HMAC-SHA1.

(That said, the hash cost may become very relevant when we eventually ship "Phabricator Valuable Golden Coin Money", our blockchain-based virtual currency.)

Apr 6 2017, 2:01 PM · Infrastructure, Security
epriestley added a comment to T12509: Plan the path forward from HMAC-SHA1.

I get these rough per-hash costs locally (Macbook Pro), with a 64-byte key:

Apr 6 2017, 1:05 PM · Infrastructure, Security
siepkes added a comment to T12509: Plan the path forward from HMAC-SHA1.

Piece of info (you guys might already be aware of it) which might be of interest when implementing this; SHA512 is often faster then SHA256 on x64. See for example: https://crypto.stackexchange.com/questions/26336/sha512-faster-than-sha256

Apr 6 2017, 12:54 PM · Infrastructure, Security
epriestley added a comment to T6994: Write a general "Security guidelines" document.

Some guidance about "configure captchas if you're a public-facing, password-login install" would be good here too, but maybe we should just raise it as a setup issue if you have password auth enabled, and let users ignore it if they're VPN'd.

Apr 6 2017, 11:49 AM · Security
epriestley closed T12512: Bug in Upload URL's (Require user authentication over URL's ) as Resolved.

In the future, please report security issues via HackerOne: https://hackerone.com/phabricator -- notably, this allows us to award you a security bounty if you discover an issue.

Apr 6 2017, 11:39 AM · Security, Bug Report
wjhussain added a project to T12512: Bug in Upload URL's (Require user authentication over URL's ): Security.
Apr 6 2017, 11:10 AM · Security, Bug Report

Apr 5 2017

epriestley added a comment to T12509: Plan the path forward from HMAC-SHA1.

I'm maybe going to try to do this in the short term:

Apr 5 2017, 8:26 PM · Infrastructure, Security
epriestley created T12509: Plan the path forward from HMAC-SHA1.
Apr 5 2017, 8:21 PM · Infrastructure, Security
cspeckmim added a comment to T12506: hackerone: possible issue with secrets sharing.

Thank you for explaining - I didn't mean to imply I thought it was a security concern, as it is very clear what giving someone Edit access means.

Apr 5 2017, 1:53 PM · Security
epriestley added a comment to T12506: hackerone: possible issue with secrets sharing.

T4721 (which I also linked on the HackerOne report) discusses usability improvements to Passphrase, including better documentation and hinting about this use case and possibly the separation of the "Can Use Credential" and "Can View Secret" policies. These are reasonable usability concerns.

Apr 5 2017, 1:16 PM · Security
cspeckmim added a comment to T12506: hackerone: possible issue with secrets sharing.

It sounds like the user is confused that having View access doesn't let them have access to decrypt/see the secret - only that they can use the secret within Phabricator. So in order to allow others to decrypt/see the secret he gives them Edit access which does give them the ability to change the secret.

Apr 5 2017, 1:09 PM · Security
cxzzero added a comment to T12506: hackerone: possible issue with secrets sharing.

Thanks for the response Evan.

Apr 5 2017, 1:04 PM · Security
epriestley closed T12506: hackerone: possible issue with secrets sharing as Invalid.

You can review the HackerOne report, including my response, here:

Apr 5 2017, 12:51 PM · Security
cxzzero created T12506: hackerone: possible issue with secrets sharing.
Apr 5 2017, 12:44 PM · Security

Mar 30 2017

Herald updated subscribers of T4340: Implement Content-Security-Policy and Strict-Transport-Security headers.
Mar 30 2017, 2:53 AM · Phacility, Security

Mar 26 2017

epriestley reopened T8918: Header shows number of notifications and various other controls on the 2FA auth screen as "Open".

(This was incorrectly closed by the text "doesn't actually fix T8918" in rP08de131da525.)

Mar 26 2017, 8:15 PM · Security, Auth

Mar 20 2017

epriestley closed T12408: Security: "Show Raw File" in Differential generated files with overbroad permissions as Resolved.

We don't plan to take any other upstream actions here, but let us know if anyone has further questions.

Mar 20 2017, 2:16 PM · Differential, Files, Security

Mar 16 2017

epriestley added a comment to T12408: Security: "Show Raw File" in Differential generated files with overbroad permissions.

(You could also pipe the list into bin/remove destroy --force, equivalently.)

Mar 16 2017, 8:22 PM · Differential, Files, Security
epriestley added a comment to T12408: Security: "Show Raw File" in Differential generated files with overbroad permissions.

You can re-run the migration explicitly with:

Mar 16 2017, 8:22 PM · Differential, Files, Security