SecurityTag
ActivePublic

Properties

Description

Tasks related to enhancing the security of Phabricator.

Recent Activity

Thu, Apr 13

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.
Thu, Apr 13, 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?)

Thu, Apr 13, 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.

Thu, Apr 13, 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.
Thu, Apr 13, 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.
Thu, Apr 13, 4:11 AM · Conpherence (v4), Security
chad claimed T12178: Users can send messages to Conpherence rooms they do not have CAN_JOIN permission for.
Thu, Apr 13, 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.

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

Mon, Apr 10

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.

Mon, Apr 10, 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.
Mon, Apr 10, 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.
Mon, Apr 10, 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:

Mon, Apr 10, 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.
Mon, Apr 10, 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.
Mon, Apr 10, 1:26 PM · libphutil, Security

Thu, Apr 6

epriestley created T12515: Upgrading: File Integrity Hashing and SHA1.
Thu, Apr 6, 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()".
Thu, Apr 6, 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.
Thu, Apr 6, 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.
Thu, Apr 6, 10:43 PM · Infrastructure, Security
epriestley added a revision to T12509: Plan the path forward from HMAC-SHA1: D17632: Rename "PhabricatorHash::digest()" to "weakDigest()".
Thu, Apr 6, 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.
Thu, Apr 6, 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.
Thu, Apr 6, 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.)

Thu, Apr 6, 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:

Thu, Apr 6, 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

Thu, Apr 6, 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.

Thu, Apr 6, 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.

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

Wed, Apr 5

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:

Wed, Apr 5, 8:26 PM · Infrastructure, Security
epriestley created T12509: Plan the path forward from HMAC-SHA1.
Wed, Apr 5, 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.

Wed, Apr 5, 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.

Wed, Apr 5, 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.

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

Thanks for the response Evan.

Wed, Apr 5, 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:

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

Thu, Mar 30

Herald updated subscribers of T4340: Implement Content-Security-Policy and Strict-Transport-Security headers.
Thu, Mar 30, 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
cspeckmim added a comment to T12408: Security: "Show Raw File" in Differential generated files with overbroad permissions.

We ran the script provided above to get an audit of at-risk files. Afterwards we upgraded our instance and the upgrade succeeded however its attempts to delete the affected files failed. The failure is due to using a local file store which is accessible to our web service account but not the phabricator phd services account (T4752). After correcting the file permissions so both accounts have appropriate access, running upgrade again doesn't seem to remove the files.

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

It is likely that the vulnerable code predates significant portions of the Files and permissions systems, and was just overlooked as these other systems upgraded and gained more powerful policy and permissions capabilities.

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

The fix is now available on master (rP7626ec0c) and stable (rP6f879559). I've upgraded this install without incident. Per above, note that upgrading destroys evidence, so you should plan any audit or response actions you want to take before upgrading.

Mar 16 2017, 5:17 PM · Differential, Files, Security
epriestley added a revision to T12408: Security: "Show Raw File" in Differential generated files with overbroad permissions: D17504: Correct an issue where "View Raw File" in Differential generated a file with overbroad permissions.
Mar 16 2017, 4:55 PM · Differential, Files, Security
epriestley created T12408: Security: "Show Raw File" in Differential generated files with overbroad permissions.
Mar 16 2017, 4:51 PM · Differential, Files, Security
epriestley added a parent task for T12046: PHPMailer RCE [CVE-2016-10033 and CVE-2016-10045]: T12404: Implement a first-party SMTP client.
Mar 16 2017, 1:06 AM · Mail, Security

Mar 3 2017

epriestley added a commit to T12313: Cloudflare leaked all HTTPS traffic on the internet into public caches ("Cloudbleed"): rP8ce25838f541: Provide "bin/auth revoke" with a revoker for Conduit tokens.
Mar 3 2017, 10:39 PM · Guides, Security
epriestley added a revision to T12313: Cloudflare leaked all HTTPS traffic on the internet into public caches ("Cloudbleed"): D17458: Provide "bin/auth revoke" with a revoker for Conduit tokens.
Mar 3 2017, 10:22 PM · Guides, Security

Mar 2 2017

epriestley closed T12313: Cloudflare leaked all HTTPS traffic on the internet into public caches ("Cloudbleed") as "Resolved".

Closing this since there doesn't seem to be any thing left that's actionable for us.

Mar 2 2017, 5:29 PM · Guides, Security

Feb 24 2017

allixsenos edited the description of T12313: Cloudflare leaked all HTTPS traffic on the internet into public caches ("Cloudbleed").
Feb 24 2017, 2:41 PM · Guides, Security
epriestley added a comment to T12313: Cloudflare leaked all HTTPS traffic on the internet into public caches ("Cloudbleed").

Broadly, there is nothing Phabricator could have done differently to anticipate or prevent this issue.

Feb 24 2017, 2:31 PM · Guides, Security