Last Comment Bug 689675 - XSS
Product: Socorro
Classification: Server Software
Component: Webapp (show other bugs)
: 2.2
: All All
: -- normal (vote)
: 2.2.5
Assigned To: Chris Lonnen :lonnen
: socorro
Depends on:
Blocks: 690146
  Show dependency treegraph
Reported: 2011-09-27 11:44 PDT by Ervis Tusha
Modified: 2012-01-03 16:49 PST (History)
14 users (show)
stephen.donner: in‑testsuite?
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Post-fix screenshot (719.14 KB, image/png)
2011-09-28 17:56 PDT, Stephen Donner [:stephend]
no flags Details

Description Ervis Tusha 2011-09-27 11:44:18 PDT"%2f%3E%3Cimg src=x onerror="alert(1)
Comment 1 Chris Lonnen :lonnen 2011-09-27 16:54:38 PDT

PHP's in_array function was erroneously returning true. The string started with an integer that was in the array of integers, and in converting it during the compare something happens to make it evaluate true. We then return the non-converted value.

This will patch it by explicitly converting to an integer beforehand, and then returning the converted value (now just the integer, without the characters following it).

Brandon: r?
Comment 2 Brandon Savage [:brandon] 2011-09-27 16:57:22 PDT
Lonnen: r+ and merged.
Comment 3 Ervis Tusha 2011-09-27 23:53:11 PDT still vulnerable
Comment 4 Robert Helmer [:rhelmer] 2011-09-28 01:34:18 PDT
minor syntax error (see crash-stats-dev), r? anyone:

(we should add a PHP linter to the jenkins steps :))
Comment 5 Robert Helmer [:rhelmer] 2011-09-28 01:42:37 PDT
(In reply to Ervis Tusha from comment #3)
> still vulnerable

This will go live on automatically once landed (which resolving the bug signals) until we do a production release. Before a production release, QA will test on stage and mark "verified".

There will be a "push bug" for 2.3 (or whatever milestone the bug is set to) which will be used to track pushing it to production.
Comment 6 Robert Helmer [:rhelmer] 2011-09-28 01:44:51 PDT
(In reply to Robert Helmer [:rhelmer] from comment #4)
> minor syntax error (see crash-stats-dev), r? anyone:
> (we should add a PHP linter to the jenkins steps :))

This is blocking QA, and I tested locally so merging myself (the XSS from comment 0 seems resolved btw).
Comment 7 Stephen Donner [:stephend] 2011-09-28 17:56:46 PDT
Created attachment 563258 [details]
Post-fix screenshot
Comment 9 Stephen Donner [:stephend] 2011-09-28 19:36:21 PDT
I think we could write a test for this; use urllib to verify that the URL returns a 200 OK, without an alert.

Note You need to log in before you can comment on or make changes to this bug.