The ' in the end of the #POC url is mistake. it should be

https://chromium-cq-status.appspot.com/v2/patch-status/x/42;alert(document.domain)/z

tandrii@: Is this something that you can help sort out?

I am guessing on the severity, setting it to Medium, because it is not obvious how dangerous this is.

I don't know how bad this XSS is. All data inside the app and its domain is 100% public. But bad forged URLs can perhaps be used to exploit some chrome vulnerability using our "reputable" app?


On the app itself: I'm not surprised. The code is effectively unmaintained and 99% untested. I don't yet know all the places where the code has to be fixed, but I'm very certain there are other bugs there.

Sergey or Alan, do you think you can help with this since you are more familiar with the code than anybody else?

The problem is the use of cgi.escape().
https://cs.chromium.org/chromium/infra/appengine/chromium_cq_status/handlers/patch_status.py
cgi.escape() sanitises against HTML injection but this is being inserted into Javascript.

We should wrap these with int() and repr() for numbers and strings respectively.

I don't repr is what we want.

>>> print repr((1, '"', 2, 'a', 3, "'"))
(1, '"', 2, 'a', 3, "'")

Also, the variables are used in both JS and HTML.

WDYT about this https://chromium-review.googlesource.com/422330?

CL 422330  deployed as https://303cc7b-dot-chromium-cq-status.appspot.com/ 

Thanks. I am downgrading the severity on the basis of that origin being a low value target for XSS.

For all cc-ed, it'd be great if you review my CL and suggests improvements OR maybe even upload a better CL.

Commented on the patch, we still need cgi.escape() when injecting into JS.
Example:
https://303cc7b-dot-chromium-cq-status.appspot.com/v2/patch-status/%3C/script%3EXSS%3Cscript%3E/1067633003/1

You are right. Updated patch.

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/98ddcabed466021e7458196dba3ec89e8a604d6e

commit 98ddcabed466021e7458196dba3ec89e8a604d6e
Author: Andrii Shyshkalov <tandrii@chromium.org>
Date: Wed Jan 04 14:36:37 2017

Improve patch_status of CQ status app.

R=alancutter@chromium.org
BUG= 676921 

Change-Id: I802998a8c012f5f13e180e304e113e24b21dd0ce
Reviewed-on: https://chromium-review.googlesource.com/422330
Reviewed-by: Sergey Berezin <sergeyberezin@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>

[modify] https://crrev.com/98ddcabed466021e7458196dba3ec89e8a604d6e/appengine/chromium_cq_status/handlers/patch_status.py
[modify] https://crrev.com/98ddcabed466021e7458196dba3ec89e8a604d6e/appengine/chromium_cq_status/templates/patch_status.html

Deployed as 98ddcabed version to both instances (incl. internal-cq-status) and highend + default services.

Also, filed issue 678254 to actually deprecate & remove this app.

Hey. thanks for the quick response and fix.

Is this eligible for HoF or bug reward?

Also there are a few other chromium related app spot applications, would it worth or okay if I take a look for vulns too? 

It's for kenrb@ to decide.

It's not really up to me, there is a panel that makes vulnerability reward decisions. Unfortunately I don't think this will get picked up because it does not meet the program requirements, described here: https://www.google.com/about/appsecurity/chrome-rewards/index.html

I'll send a note to the reward program manager to see if he wants to flag this for panel review, but low severity vulnerabilities are typically not considered.

Thanks for the report and the fix. (:

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot