CSV export vulnerable to formulae injection (again)

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Query/Bug List
--
minor
RESOLVED FIXED
a year ago
9 months ago

People

(Reporter: Muhammed Gamal, Assigned: Frédéric Buclin)

Tracking

unspecified
Bugzilla 4.4
Bug Flags:
approval5.0 +
approval4.4 +
sec-bounty -

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36

Steps to reproduce:

file a new bug with name "-2+3+cmd|' /C calc'!A0" and continue the needed information


Actual results:

when the bugs exported as a csv file , it the cmd code will run poping up the calculator


Expected results:

filtering the inputs
(Assignee)

Updated

a year ago
Assignee: import-export → query-and-buglist
Component: Bug Import/Export & Moving → Query/Bug List
(Assignee)

Updated

a year ago
Duplicate of this bug: 1263581
(Reporter)

Comment 2

a year ago
Are there any news ?
Flags: needinfo?(LpSolit)
(Assignee)

Comment 3

a year ago
Not for now, no.
(Assignee)

Updated

a year ago
Flags: needinfo?(LpSolit)

Comment 4

a year ago
hi

this bug #1259881 and #1054702 Is the same issue ! (both are using same payload and method ("-2+3+cmd|' /C calc'!A0")
 i tested #1259881 (this issue which is reported by muhammed gamal)  and UNABLE TO REPRODUCE  that using "-2+3+cmd|' /C calc'!A0" .


so i tried with %0A-2+3+cmd|' /C calc'!D2 and its working with new Line unicode charcter !
can you please take a look again ! more details i mentioned in my report bug #1263581


Thanks
(Reporter)

Comment 5

a year ago
Hello Paresh ,

in the report you mentioned the reporter didn't use my payload he used "=" to perform actions .

in my report , of course it works with that's why i report it !

here is a Picture to Proof My concept
http://i.imgur.com/PLPmsQN.png

Comment 6

a year ago
ok! 

i tried again , still unable to reproduce your issue! ( tried in exel )

Anyways
i hope mozilla/bugzilla team checks both issues well !


Regards !
(Reporter)

Comment 7

a year ago
As u can see it is working with me in excel too ( excel 2013 version )

i hope you the best too !
(Assignee)

Updated

a year ago
Duplicate of this bug: 1264660
I don't see how this is a Bugzilla issue.  This sounds like a bug in Excel to me.

Comment 10

a year ago
(In reply to Dave Miller [:justdave] (justdave@bugzilla.org) from comment #9)
> I don't see how this is a Bugzilla issue.  This sounds like a bug in Excel
> to me.

Yup! but also bugzilla issue.
This issue isn’t specific to web applications or any particular file format – any situation where untrusted content ends up in a spreadsheet could be exploited.

For fix:
Sanitise user input. There would not be a problem in the first place if the application were sanitising user input. Characters such as ‘=’ should not be allowed in fields such as  report Title. If these characters are to be allowed, a recommended fix is to append a single quote (‘) to the list of formula triggers ( =, +, -) before saving it in the database. LibreOffice or Excel will ignore the single quote and just show the malicious formula as a string i.e. it won’t be interpreted as a formula.


btw ive found 2 year old report, which is fixed https://bugzilla.mozilla.org/show_bug.cgi?id=1054702 

and this report same as Bug 1054702
OK, so this is a duplicate of bug 1054702 and it's already fixed then?
(Reporter)

Comment 12

a year ago
Nope , in bug 1054702 it's fixed and the reporter already got his bounty , i have just bypassed the fix
I notice your screenshot has an "Enable macros" dialog in it.  Did you have to enable macros for this exploit to work?

Comment 14

a year ago
(In reply to Muhammed Gamal from comment #12)
> Nope , in bug 1054702 it's fixed and the reporter already got his bounty , i
> have just bypassed the fix

How you bypassed the fix ?
in both report (yours and  bug 1054702) using same method (=) .

it would be nice if you provide Video Proof of concept.
because i tried it and (=) doesnt execute. 

Thanks
(Reporter)

Comment 15

a year ago
i don't know where is "=" in "-2+3+cmd|' /C calc'!A0" !!!

Comment 16

a year ago
(In reply to Muhammed Gamal from comment #15)
> i don't know where is "=" in "-2+3+cmd|' /C calc'!A0" !!!

Thats the point, how you executing this "-2+3+cmd|' /C calc'!A0"   Without (=) ? 
Thats why i am asking for video proof of concept.
(Reporter)

Comment 17

a year ago
This is a Unlisted POC Video
https://youtu.be/2OuQ8Awkq88

Comment 18

a year ago
(In reply to Muhammed Gamal from comment #17)
> This is a Unlisted POC Video
> https://youtu.be/2OuQ8Awkq88

Thanks,
next time you test anything, test on https://landfill.bugzilla.org/bugzilla-5.0-branch/ (test server)
(Reporter)

Comment 19

a year ago
OK , i knew after i tested it , and i removed the bug report .
i hope that the video provide the information you need to know
The video shows Excel saying "This is dangerous, don't do it" and you overriding it to do it anyway.

"Microsoft Office has identified a potential security concern.

File Path: C:\Users\Mohammed\Downloads\bugs-2016-04-15.csv

Automatic update of links has been disabled. If you choose to enable automatic update of links, your computer may no longer be secure. Do not enable this content unless you trust the source of this file."
(Reporter)

Comment 21

a year ago
CSV Injection is one of Command Injection bugs and has a critical Impact 
as u can see at "https://bugzilla.mozilla.org/show_bug.cgi?id=1054702"
the Bugzilla platform was Vulnerable to this bug 2 years ago , and i am just bypassed the fix
and if you see that's not a bugzilla bug you can check 
"https://bugzilla.mozilla.org/show_bug.cgi?id=1054702#c7"
See Also: → bug 1264939
(Assignee)

Updated

a year ago
See Also: bug 1264939
What characters in that string are triggering it?  I see nothing in that string that I wouldn't want to allow in a bug summary, to be honest, so if we're going to do anything, we need to know what the trigger actually is.

However....   The fact that Excel triggers a warning when you open the file makes me care way less about this one.  The previous bug you're citing was a case where you could execute things from loading a csv file without any warning when you opened it.  This one, Excel actually attempts to prevent you from running the dangerous code.  It's not really our problem if you ignore the warnings.
This bug is not eligible for a bug bounty -- the dangerous functionality is clearly on the Excel side (and somewhat mitigated). This attack supposes that someone will download a CSV of a buglist, open it, and then think it is perfectly normal for a Bugzilla buglist to be launching external functionality.
Flags: sec-bounty-
(Reporter)

Comment 24

a year ago
In the other report The researcher tested it in LibreOffice , and if you tested it in it or any CSV reading file it will be run without warning .
and if you tried the old one with Excel it will be warned also !
I think command execution possibility should be get some more attention even with warning.
most of big companies know the impact of the Vulnerability even with Excel warnings 
there are some reports of Big Companies and has warning box also ! 
Hackerone : https://hackerone.com/reports/72785 , https://hackerone.com/reports/111192
Zendesk : https://hackerone.com/reports/90131
Uber : https://hackerone.com/reports/126109

and about the word you should triggering , CMD , HYPERLINK and all words can be executed in the CSV file
(Reporter)

Comment 25

a year ago
@Daniel 
please check what you said in the old report !
https://bugzilla.mozilla.org/show_bug.cgi?id=1054702#c7
I should note that we weren't happy paying a bounty for bug 1054702 either -- it's still an excel bug exposing such dangerous functionality, but in that case the unbelievable stupidity of excel launching external programs without prompt put users at enough risk that it was worth trying throw some blocks up.
(Reporter)

Comment 27

a year ago
it's not excel only which this works for , any csv files reader will execute the command , you have to know that there are alot of people use different software to read it .
and the big companies don't give a chance to harm their user through this vulnerability so it filtering their inputs from this commands .
i think this case should be considered as the other one !
If I may suggest a patch, you guys could add an escape("\x20") before the bugs summary when it starts with "-" and "+", as you already do when it begins with "=". Do it like this:
=========================
"Bug ID","Product","Component","Assignee","Status","Resolution","Summary","Changed"
1120074,"Bugzilla","User Accounts","user-accounts@bugzilla.bugs","UNCONFIRMED","---"," -SOME-CMD-CODE","2015-08-20 07:12:39"

Putting the space will prevent it from being interpreter as a macro.
Ops, what I meant "space" not scape :P
Looks like bug 1054702 only patched '=' and we ought to have escaped -+@ as well. See "Update (April 2016)" at http://www.contextis.com/resources/blog/comma-separated-vulnerabilities/ (triggered by
https://twitter.com/albinowax/status/717642300961255425 )

Also an initial ' might have been a better escape than a space.
Note that blog and tweet came from the reporter of bug 1054702
(Reporter)

Comment 32

a year ago
i think this is strengthening opening the case again !
(Assignee)

Comment 33

a year ago
Blindly interpreting everything is an impressive stupidity from Microsoft & co. The CSV RFC (rfc 4180) doesn't mention this possibility, see section 3:

"Security considerations: CSV files contain passive text data that should not pose any risks."

It's totally Excel's fault to execute code coming from a CSV file. Note that Excel is aware of this as it throws 2 clear warnings. But it shouldn't throw these warnings. It should simply ignore "special" characters coming from CSV files. Period.


As we already treat "=" specifically, we can simply add +, - and @ to the regexp. But this is *not* a Bugzilla bug, to make things clear.
Status: UNCONFIRMED → NEW
Ever confirmed: true
See Also: → bug 1054702
Summary: csv injection in bugs list → CSV export vulnerable to formulae injection (again)
Target Milestone: --- → Bugzilla 4.4

Comment 34

a year ago
I totally agree with Muhammed Gamal! 
This bug is not an Excel bug, it's a Bugzilla bug because the bug will affect Bugzilla users and Bugzilla is the only application who can fix this and protect the users.
And as Muhammed mentioned, other bug companies agree that CSV injection is a web-application vulnerability and have nothing to do with Excel, And they also reward bounties for it and take it seriously.

Comment 35

a year ago
(In reply to Frédéric Buclin from comment #33)
> Blindly interpreting everything is an impressive stupidity from Microsoft &
> co.

Haha !
Please someone add Jonathan or jesse or @shurik41 from 'Microsoft Security Response Center'  here :D
Google has a very good statement about "csv injection" severity: https://sites.google.com/site/bughunteruniversity/nonvuln/csv-excel-formula-injection
"""
CSV files are just text files (the format is defined in RFC 4180) and evaluating formulas is a behavior of only a subset of the applications opening them - it's rather a side effect of the CSV format and not a vulnerability in our products which can export user-created CSVs. This issue should mitigated by the application which would be importing/interpreting data from an external source, as Microsoft Excel does (for example) by showing a warning. In other words, the proper fix should be applied when opening the CSV files, rather then when creating them.
"""
(Assignee)

Comment 37

a year ago
(In reply to Paresh from comment #35)
> Please someone add Jonathan or jesse or @shurik41 from 'Microsoft Security
> Response Center'  here :D

I don't care about their opinion, really. They interpret passive text as formula, fine. Their problem!

Comment 38

a year ago
(In reply to Mario Gomes from comment #36)

I kinda agree with what you've said.. But then why did you guys fix this one #1054702?
As I can see, It's almost the same!
(Reporter)

Comment 39

a year ago
I think the important is the safty of the users not who is responsible for , and when you can avoid that and you did that before , why don't you do the same here ?!
(Assignee)

Comment 40

a year ago
(In reply to Muhammed Gamal from comment #39)
> I think the important is the safty of the users not who is responsible for ,
> and when you can avoid that and you did that before , why don't you do the
> same here ?!

I didn't say we won't do it (I marked the bug as NEW, didn't I?), though we can still decide to close the bug as WONTFIX later. What I'm saying is that due to Microsoft inability to follow RFCs, other applications are forced to take some actions to prevent some undesirable behaviors in Excel.
(Assignee)

Comment 41

a year ago
Created attachment 8741913 [details] [diff] [review]
patch, v1

Simon: you wrote the original code to catch "=". Want to test this one?
Attachment #8741913 - Flags: review?(mail)
space-escaping makes me worry some helpful filter is going to trim and then we're back where we started. I suppose if Excel import doesn't trim then we've solved the immediate problem, but I'd rather have uglier summaries (starting with ') in this rare case than have to worry.

'rare' in relative terms, BMO has 1178 bugs matching short_desc=^[-+=@]&short_desc_type=regexp (about one in a thousand)
(Reporter)

Comment 43

a year ago
does that back the case to Flags: sec-bounty?

Updated

a year ago
Attachment #8741913 - Flags: review?(mail) → review+

Updated

a year ago
Assignee: query-and-buglist → LpSolit
(Assignee)

Comment 44

a year ago
(In reply to Daniel Veditz [:dveditz] from comment #42)
> space-escaping makes me worry some helpful filter is going to trim and then
> we're back where we started.

I agree with that, but in bug 1054702 sgreen said the single quote (correctly) remained visible in LibreOffice, and glob also asked to use a whitespace instead. Honestly, there is no reason for an application importing a CSV file to remove leading quotes (I call this data corruption), and so Excel again behave incorrectly. The whitespace is a compromise between readability and preventing Excel from executing formula.


> I suppose if Excel import doesn't trim then
> we've solved the immediate problem, but I'd rather have uglier summaries
> (starting with ') in this rare case than have to worry.

I would rather think the other way. If this fixes the problem with the Excel importer, then we did our job, and I don't think we should make another application's life harder due to leading quotes which are not part of the original data.
Flags: approval?
Flags: approval5.0?
Flags: approval4.4?
(Reporter)

Comment 45

a year ago
Updates ?

Comment 46

a year ago
@Tazuwk: See previous comments in this task. 
If you have a specific question to somebody, please ask it. Thanks.
(Reporter)

Comment 47

a year ago
Does all of that make the bug eligable for bounty ?!
Flags: needinfo?(dveditz)
Flags: needinfo?(LpSolit)
(In reply to Muhammed Gamal from comment #47)
> Does all of that make the bug eligable for bounty ?!

this issue has already been marked as not eligible, regardless of the approach taken to mitigate excel's issue.
Flags: needinfo?(dveditz)
Flags: needinfo?(LpSolit)
(Reporter)

Comment 49

a year ago
I still think it should be eliagble for bounty , the issue is many attack ma use your website for attack mozilla users ! 
If it's not critical bug why did u pay bounty for it in the past report ! .
Many bug compaines pay good bounties for bug like this !
And of course mozilla is a big one !
(In reply to Muhammed Gamal from comment #49)
> I still think it should be eliagble for bounty

please take any further discussion of the bug's bounty eligibility to security@mozilla.org, and not on this bug.

thank you.

Comment 51

a year ago
(In reply to Muhammed Gamal from comment #49)
> I still think it should be eliagble for bounty , the issue is many attack ma
> use your website for attack mozilla users ! 
> If it's not critical bug why did u pay bounty for it in the past report ! .
> Many bug compaines pay good bounties for bug like this !
> And of course mozilla is a big one !

If you want to discuss about Bounty mail here: security@mozilla.org  .

Comment 52

a year ago
there's another way to execute commands in excel using CSV.
Problem described here: http://stackoverflow.com/questions/2668678/importing-csv-with-line-breaks-in-excel-2007
(Reporter)

Comment 53

a year ago
i sent an email to Security@mozilla.org and still has no answer , i still don't know how it is now not a critical issue and it was that in the past report ! , i could use this vulnerability to harm alot of bugzilla members which is a the place where all security reporter comes , and that means i will get these reports all and the reporters machines too !
(Assignee)

Comment 54

a year ago
(In reply to Muhammed Gamal from comment #53)
> i sent an email to Security@mozilla.org and still has no answer , i still
> don't know how it is now not a critical issue and it was that in the past
> report ! , i could use this vulnerability to harm alot of bugzilla members
> which is a the place where all security reporter comes , and that means i
> will get these reports all and the reporters machines too !

Please stop complaining about this bug not being eligible for a bounty. The reasons have already been written several times, especially in comment 20, comment 22 and comment 23: Excel throws warnings about potential security issues when enabling formula execution, and you decided to ignore these warnings! You have been warned! This is what makes this bug report different from bug 1054702, where code was automatically executed.

Also, as mentioned in comment 33 and comment 36, executing data coming from a CSV file is not specified in RFC 4180. This is an issue with the CSV file reader, not the CSV file generator.
Flags: approval5.0?
Flags: approval5.0+
Flags: approval4.4?
Flags: approval4.4+
(Assignee)

Comment 55

a year ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   e2b1136..ff441bc  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   ea0c502..8b0d558  5.0 -> 5.0

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   94d623c..36253a7  4.4 -> 4.4
Severity: normal → minor
Status: NEW → RESOLVED
Last Resolved: a year ago
Flags: approval?
Resolution: --- → FIXED
(Assignee)

Updated

11 months ago
Duplicate of this bug: 1279878
(Assignee)

Updated

9 months ago
Duplicate of this bug: 1290577
You need to log in before you can comment on or make changes to this bug.