Page MenuHomePhabricator

Unknown Objects
Closed, ResolvedPublic

Description

I noticed a bunch of unknown objects on T9399, not sure what happened there...

Screenshot_2015-09-12-11-07-45.png (1×1 px, 324 KB)

Event Timeline

joshuaspence raised the priority of this task from to Needs Triage.
joshuaspence updated the task description. (Show Details)
joshuaspence added a project: Maniphest.
joshuaspence added a subscriber: joshuaspence.
123123123 edited subscribers, added: joshuaspence; removed: devurandom.
epriestley claimed this task.
epriestley added a subscriber: epriestley.

I suspect this is a "security researcher" manually editing the form to submit bogus values. We've had some activity on HackerOne recently which puts this project in various feeds there, and there are a large number of "security researchers" who do not read the program description at all before attempting to grab some quick cash by finding trivial XSS.

My general thinking on these is:

  • We must handle bogus values during reads gracefully if they're in the database, since we can't prevent them from appearing there (in the base case, administrators can go change the database values manually). The behavior on T9399 satisfies this in my view.
  • We should handle bogus values during queries if they violate policies. For example, it's not allowable for a user to be able to search for tasks in a project they can't see. This discloses information they can not otherwise access, so we should prevent it.
  • Outside of these cases, I'm not hugely concerned about permitting invalid writes. It would be nice to prevent them, but we don't have ways to do this systematically right now, and I'd rather this just happen than we copy/paste a ton of validation code into every application. After T9132, we may have better systematic ways to prevent these kinds of things. We should prevent these once we're reasonably able to do so in a consistent, non-copy-pastey way.

Here's an example of one of the transactions the user submitted:

*************************** 5. row ***************************
             id: 136665
           phid: PHID-XACT-TASK-wwgawyguobp73fo
     authorPHID: PHID-USER-5thlbzlp5ftp5dhc4fgg
     objectPHID: PHID-TASK-wpypeexuucb6uvnycv75
     viewPolicy: public
     editPolicy: PHID-USER-5thlbzlp5ftp5dhc4fgg
    commentPHID: NULL
 commentVersion: 0
transactionType: reassign
       oldValue: "PHID-USER-tcxolim2ajn4caom7r2h"
       newValue: "\"><img src=x onerror=prompt(document.domain);>"
  contentSource: {"source":"web","params":{"ip":"<redacted>"}}
       metadata: []
    dateCreated: 1441993375
   dateModified: 1441993375

Note that newValue contains an XSS effort, so I think this is consistent with a "security researcher" using "Inspect Element..." to muck with the form.

I'm just going to close this since I don't plan to take any specific action outside of infrastructure improvements described in T9132, which will eventually lead to better universal validation (hopefully). I consider the overall behavior of the system in this situation to be reasonable for now (no explosions, no policy violation).