962362 - IARC prod tool throws XSS error for some developers

Status: VERIFIED FIXED

(Marketplace Graveyard :: Developer Pages, defect, P1)

Description

[Christopher Van Wiemeersch [:cvan]]

From a developer via the app-reviewers mailing list:

> When I click on the Developers  My Submissions Pay Ontime Content Ratings -->  Create an IARC Ratings Certificate button the page throws error. Please refer the attachment for further details

Can someone investigate and/or contact the IARC?

Comment 1

[Christopher Van Wiemeersch [:cvan]]

Screenshot:
http://f.cl.ly/items/3K0P362U2p0f0h3Q1B0E/bug.png

Comment 2

[Kevin Ngo [:ngoke]]

Christine, it seems like a developer has angle brackets in their company name and is not accepted as valid input in the IARC tool.

Comment 3

[Kevin Ngo [:ngoke]]

It works for my apps on prod, maybe since I don't have an angle bracket in my developer name.

Comment 4

[Christine McCarthy]

I will  keep look at whether we can yank the .net built-in form security reasonably safely to allow for angle brackets. If not, we may have to parse and encode angle brackets when submitting them. I will look at it first thing in the morning.

Comment 6

[Rob Hudson [:robhudson]]

FWIW, in the form the angle brackets are escaped, e.g.:

  <input type="hidden" name="company" value="Robérto &lt;[e-mail]&gt;">

Comment 7

[Christine McCarthy]

Rob, I've managed to recreate this on our end. When I pass the string as you indicate above (with the brackets escaped) it seems to go through cleanly. But when I pass angle brackets themselves, the error occurs. We will be looking at how we can get this addressed quickly and will keep you updated.

Comment 8

[Christine McCarthy]

Here is the current situation, and I'd like a sense from your end on which way we should go.

1. We don't have a quick solution to accepting unencoded angle brackets, but are continuing to work on finding one.

2. We could remove validation from the page, which would allow unencoded angle brackets through, but would also open us to some potential security issues, and so would have to be a temporary measure, but could be implemented quickly.

3. In our testing, HTML encoded angle brackets submit cleanly regardless.

So, the next move really depends on whether this is preventing your users from being able to make their submissions. If it is, we can put the temporary fix in place while we work out the permanent fix. I would also be interested in having you test the encoded vs. decoded brackets, since Rob said you were already sending them encoded, and we didn't have any problems when we were encoded.

Comment 9

[Rob Hudson [:robhudson]]

We're encoding the values in the form but when they are POSTed they get converted to true angle brackets. We can double encode them so you actually get the "&gt;" and "&lt;" values. The only problem is this isn't representative of what we actually store on our end, which is "Rob <[e-mail]>". It's only HTML encoded so it doesn't cause XSS problems when displayed in HTML.

Alternatively could you not HTML encode this form field on your side? Or does the framework security kick in before you even get the data?

Comment 10

[Christine McCarthy]

We're looking into exactly where the framework security kicks in, we're not currently sure if it's the moment the form arrives with angle brackets, or not until we start trying to use the form variables. That's the permanent fix we're currently hoping to put in place.

We are comfortable with having a slight variation in our stored data, as the encoded values should continue to display correctly when printed to screen, if you'd like to try double encoding to get everything working quickly. We will continue pursuing answers for the unencoded angle brackets regardless.

Comment 11

[Rob Hudson [:robhudson]]

https://github.com/mozilla/zamboni/commit/12f1849 

To verify, submit an app with developer.name in the manifest containing HTML characters (e.g. "Rob <[e-mail]>"). On the content ratings step of the submission process, view page source and ensure that the "company" field is double escaped.

Comment 12

[Iulian Timis]

Verified as fixed in https://marketplace-dev.allizom.org/developers/ on FF29 (Win 7).
Attaching postfix screenshot.
Closing bug.

Comment 13

[Iulian Timis]

Attachment: postfix_html_escaped.png