619467 - Arbitrary PHP Execution / XSS on support.mozilla.com

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Neal Poole]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: 

I discovered that it is possible to upload arbitrary PHP scripts to support.mozilla.com using image upload facilities. A malicious PHP script can be embedded in the "comments" of a GIF: the file itself will be a valid GIF but the PHP parser will treat the code inside it as valid.

My first attempt was to upload a file via the profile image upload system: I was able to upload a file with a .php extension (I shouldn't be allowed to do that). However, the system converts files from GIF to JPEG, which wiped out my comments. I didn't take the time to see if there was a way to get around that system.

My next (and successful) attempt was via the "Add an image to a question" functionality. I submitted a POST to https://support.mozilla.com/en-US/upload/image/questions.Question/770771 containing a malicious but valid GIF file (favicon12345.php) with embedded PHP (<?php passthru('whoami') ?>). The file also included a simple XSS test (<script>alert('a')</script>). To my surprise, the file uploaded without any issues and I could browse to it and see the results (an alert box and "apache" in the text of the GIF). It would be trivial for me to turn this exploit into a remote shell into the system (replace 'whoami' with $_GET['c'] in the example above)

Reproducible: Always

Steps to Reproduce:
1. Find GIF file
2. Use Imagemagick or similar utility to add PHP code
3. Rename file to end in .php
4. Upload to support.mozilla.com
5. Browse to the file directly, see code execution

Comment 1

[David Chan [:dchan]]

Investigating issue

Comment 2

[Neal Poole]

The media gallery feature is also vulnerable. I uploaded the same proof of concept: https://support.mozilla.com/media/uploads/gallery/images/favicon.php

Comment 3

[James Socol [:jsocol, :james]]

We don't need to execute PHP anymore, can we just remove mod_php?

Comment 4

[Reed Loden [:reed]]

This should be fixed now.

(In reply to comment #3)
> We don't need to execute PHP anymore, can we just remove mod_php?

That depends on if those machines are completely dedicated to SUMO or not, I would think...

Comment 5

[James Socol [:jsocol, :james]]

Neal, can you upload the gif you used?

Comment 6

[Neal Poole]

Attachment: Favicon with PHP/XSS

You can use the Imagemagick CLI to generate your own malicious image using the following command:

convert -comment "HTML/PHP GOES HERE" /path/to/src /path/to/dest

e.g.
convert -comment "<script>alert('a')</script> <?php passthru('whoami') ?>" favicon.ico favicon.jpeg

Comment 7

[Reed Loden [:reed]]

Reopening, as clyon wants to see if there's anything that can be done at the SUMO app level to better resolve this, even though apache config options are handling it at the server level for now. For one, why are files with .php allowed to be uploaded?

Comment 8

[Jeremy Orem [:oremj]]

Yeah, it should be fixed on the client side as well, so it isn't vulnerable if someone else uses the code. I didn't notice this bug was filed against SUMO and not ServerOps.

Comment 9

[Jeremy Orem [:oremj]]

/me comments on wrong bug. Ignore comment 8!

Comment 10

[James Socol [:jsocol, :james]]

(In reply to comment #4)
> This should be fixed now.

That "fix" refers to bug 619477 comment 3, which addresses the server-side execution and should address client-side execution, or most of it.

> (In reply to comment #3)
> > We don't need to execute PHP anymore, can we just remove mod_php?
> 
> That depends on if those machines are completely dedicated to SUMO or not, I
> would think...

The sumo cluster (pm-app-sumo0*) is dedicated, yes.

We are currently writing fixes for all image upload paths that will:
* restrict accepted files to images with allowed extensions, and
* pull the binary image data out of the image and resave the file.

Comment 11

[Reed Loden [:reed]]

(In reply to comment #10)
> We are currently writing fixes for all image upload paths that will:
> * restrict accepted files to images with allowed extensions, and
> * pull the binary image data out of the image and resave the file.

Sounds great! Will these be middleware/commonware type things that can be reused by multiple apps?

Comment 12

[James Socol [:jsocol, :james]]

(In reply to comment #11)
> Sounds great! Will these be middleware/commonware type things that can be
> reused by multiple apps?

For today we're just solving this issue. We'll look at pulling it out later.

Comment 13

[Paul Craciunoiu [:paulc]]

Extensions are now restricted for images, here:
https://github.com/jsocol/kitsune/commit/39505dd6479be469db64d25831ab841c429656c1

Comment 15

[Rebecca Billings [:rbillings]]

I downloaded the attachment. I uploaded it into the gallery on master using Live HTTP headers. I saw 'http://safebrowsing-cache.google.com/safebrowsing/rd/ChFnb29nLXBoaXNoLXNoYXZhchABGL6ABCC-gAQyBT4AAQAB

GET /safebrowsing/rd/ChFnb29nLXBoaXNoLXNoYXZhchABGL6ABCC-gAQyBT4AAQAB HTTP/1.1
Host: safebrowsing-cache.google.com
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.9,fr-FR;q=0.7,fr;q=0.6,it-IT;q=0.4,it;q=0.3,en;q=0.1
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 115
Connection: keep-alive
Cookie: PREF=ID=d3768c14a6311ba9:U=a3649977ab0c4198:FF=0:TM=1292284892:LM=1292355620:S=wJZDomcXFjfbdW0P; NID=41=JlJbZ_skBNqpfSzbwvmsGrRAwHKz7ZIaMU9he62kwd5Pmbih5gvdHlktV4cZJ4tr2fPPVtlj5_w7fsAlNIi_nIcAJ4mxdWYaTvkau_HXh916LTiOtwOYnv36fCru0mMb; SID=DQAAAKsAAAC6jIpW7VAva5qoVoB7CaRejuqprJxjKPKnrk3d16no4fNZcDnyLbD8ggBR417nsxaPAb9mhQxdcWaw5gqbhVr_5qOrtMOyUSefTcnCqRIgdq4QxItIpFwCMjRFi1u7QRBfoeSS4RNRfoNQhZ2pCfTAZnQ3wYgMRv6h62wXAWuCf3WNuPt3t7plC2jlTjknqPiFtxYju-ElLw7yfa0E-QmZ7-obSRoqOYXetPB1N-0qRg; HSID=AMgJudsZFIS8zZn-j
Pragma: no-cache
Cache-Control: no-cache
HTTP/1.1 200 OK
Content-Type: application/vnd.google.safebrowsing-chunk
X-Content-Type-Options: nosniff
Date: Thu, 16 Dec 2010 03:49:38 GMT
Server: Chunked Update Server
Content-Length: 40
X-XSS-Protection: 1; mode=block'

I'm guessing this is sufficient to check- but please let me know if other steps should be taken. If not, please resolve and I'll mark it as verified.

Comment 16

[Neal Poole]

I don't think the Safebrowsing stuff is related, but I can confirm that I'm not able to upload a file with a .php extension anymore (I tested it using LiveHTTPHeaders as well).

Comment 17

[Chris Lyon [:clyon]]

yep, this issue is resolved, I think we do want to see a few more things done but for the most part, we are good. 

@james, can we close this and open separate bugs for the items we have discussed? Just want to make sure that we don't forget.

Comment 18

[Paul Craciunoiu [:paulc]]

Chris: I filed bug 619629, and we already had bug 596116 for renaming uploaded files.

IIRC from what James said, this bug and those other 2 cover the main things you mentioned.

Closing this bug.

Comment 19

[Paul Craciunoiu [:paulc]]

Landed on 2.4.x branch:
https://github.com/jsocol/kitsune/commit/23faad4f91d6268924d08b0411acc0305b0f6897

Comment 20

[Daniel Veditz [:dveditz]]

The reporter has asked about whether he can disclose this bug now. The fix has been deployed so it appears OK, but paulc filed a couple of dependent bugs (comment 18) so maybe there's more work we'd like to do before potentially putting people at risk from related bugs.

Comment 21

[Neal Poole]

That's me: thanks for the clarification :)

Comment 22

[Paul Craciunoiu [:paulc]]

I'd wait another day, we're pushing a minor security fix today (bug 619603). However, the important part was making sure the server sets proper mime type headers, and mcoates says we don't have any other pressing related issue at this time.

I'd like to wait until tomorrow to disclose this.

Comment 23

[Neal Poole]

Please, take as long as you need. I'm not in any hurry: I asked the question because I didn't want to step on anyone's toes by mentioning the vulnerability before people were ready for it to be disclosed. :)

Comment 24

[James Socol [:jsocol, :james]]

I think we're probably OK to disclose this now, the fix is live and has been. Though I'd be even happier to do it if bug 619603 was verified.

Comment 25

[Yvan Boily [:ygjb][:yvan]]

Adding keywords to bugs for metrics, no action required.  Sorry about bugmail spam.