Closed Bug 323990 Opened 14 years ago Closed 13 years ago

sql injection in talkback-public.mozilla.org.

Categories

(mozilla.org :: Talkback Server & Webtool, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: guninski, Assigned: jay)

References

Details

there is sql injection in talkback-public.mozilla.org.

unbalanced quotes:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=feck&vendor=M'ozillaOrg&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

DB Error processing query: SELECT d.bbid from blackboxes_by_deployment d, fc__keytable_1048 s WHERE lower(s.keyvalue) LIKE '%feck%' AND d.bbid = s.bbid AND d.keyvalue LIKE 'M'ozillaOrg%%%' ORDER BY d.bbid DESC


false condition:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=feck&vendor=M'%20AND%202%3E1%20AND%20'feckbill'%20=%20'ozillaOrg&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

0 crashes found where the Stack Signature contains 'feck' and the Deployment ID looks like 'M' AND 2>1 AND 'feckbill' = 'ozillaOrg%%%'.

*** This bug has been marked as a duplicate of 258484 ***
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
iirc very similar (probably the same bug) got fixed in the past but obviously has resurrected.
Group: security → webtools-security
This is still a problem and describes the current issue much better than bug 258484, reopening.

Why have we not fixed this one? We can't afford to have the talkback database corrupted or deleted -- or does the site operate on a read-only copy of the data?
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
I'm guessing the reporting side (talkback-public.mozilla.org) uses read-only, but that is just a guess.  The digesters/collector side of the backend is the only part of the system that might need write privs.  I guess the thing to check in that scenario would be if someone could do injection by submitting a false/modified blackbox that spoofed an incoming crash report.

shiva, do you have cycles to look at this, or schrep, could we get ctuft to look at it?
I don't know if we have removed all the cold fusion stuff, or how much it might be  exposed to public access but these were also reported by secuina today

http://secunia.com/advisories/21866/
http://secunia.com/advisories/21858/
(In reply to comment #4)
> I guess the thing to
> check in that scenario would be if someone could do injection by submitting a
> false/modified blackbox that spoofed an incoming crash report.
> 

i suspect that some sql servers allow OS command execution via (twisted) SQL.

and "false talkback incidents" are very easy to generate with gdb/ptrace(2) so no need to inject them via sql imho.


window loves year old sql injection on corporate boxen
ispiked: Can you look into this?
OS: Linux → All
Hardware: PC → All
Taking, I'll look into this and see if Adam can help me.

To answer the questions raised in comment #3 and comment #4, the public query interface only has read access to the database, so we don't have to worry about messing up the data.

Regarding comment #5, the CF stuff is all internal, and not publicly accessible, so not a critical issue for us.

Now to the real issue, I don't have much experience with sql injection bugs, so will try my best to figure things out.  If anyone has a working testcase I can work with to understand the problem during my debugging, that will be great!  The original comment #0 give me something to go with, but anything more will be nice.  Georgi?
Status: REOPENED → ASSIGNED
Duplicate of this bug: 258484
jay, comment 0 has 2 testcases:
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=feck&vendor=M'ozillaOrg&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid
DB Error processing query: SELECT d.bbid from spiral.blackboxes_by_deployment d, spiral.fc__keytable_1048 s WHERE lower(s.keyvalue) LIKE '%feck%' AND d.bbid = s.bbid AND d.keyvalue LIKE 'M'ozillaOrg%%%' ORDER BY d.bbid DESC

http://talkback-public.mozilla.org/search/start.jsp?search=1&searchby=stacksig&match=contains&searchfor=feck&vendor=M'%20AND%202%3E1%20AND%20'feckbill'%20=%20'ozillaOrg&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid
0 crashes found where the Stack Signature contains 'feck' and the Deployment ID looks like 'M' AND 2>1 AND 'feckbill' = 'ozillaOrg%%%'

introduction to sql injection is at:
http://en.wikipedia.org/wiki/Sql_injection

about fixing this:

if possible, don't construct a single string with the sql statement. instead build a string with placeholders for the arguments and pass the arguments.

example from wiki:
$query = $sql->prepare("select * from users where name = ?");
$query->execute($user_name);



in case the description is clear enough.

the simplest case of sql injection is using single quote ' or double
quote "


the 1st uri is:
http://talkback-public.mozilla.org/search/start.jsp?search=1&searchby=stacksig&match=contains&searchfor=feck&vendor=M'ozillaOrg&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

the page is:
DB Error processing query: SELECT d.bbid from spiral.blackboxes_by_deployment d, spiral.fc__keytable_1048 s WHERE lower(s.keyvalue) LIKE '%feck%' AND d.bbid = s.bbid AND d.keyvalue LIKE 'M'ozillaOrg%%%' ORDER BY d.bbid DESC
observe this:
'M'ozillaOrg%%%'

3 unbalanced single quotes caused by:
vendor=M'ozillaOrg
observe the single quote.

the second url is:
http://talkback-public.mozilla.org/search/start.jsp?search=1&searchby=stacksig&match=contains&searchfor=feck&vendor=M'%20AND%202%3E1%20AND%20'feckbill'%20=%20'ozillaOrg&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid

it returns:
0 crashes found where the Stack Signature contains 'feck' and the Deployment ID looks like 'M' AND 2>1 AND 'feckbill' = 'ozillaOrg%%%'

observe there is no db error and the 6 single quotes are balanced.
from the uri:
vendor=M'%20AND%202%3E1%20AND%20'feckbill'%20=%20'ozillaOrg
(url encoded, %20 is space)





(In reply to comment #11)
> if possible, don't construct a single string with the sql statement. instead
> build a string with placeholders for the arguments and pass the arguments.

which is fine until you discover these things are all written in java.  eek.

Not sure if the SQL stuff in java supports placeholders.  If it does it should make this easy to fix.
(In reply to comment #13)

> which is fine until you discover these things are all written in java.  eek.
> 

well, imho both the talkback client and server are not masterpieces of warez having in mind open source alternatives exist at least for linux.

won't be surprised if the talkback client may help exploitability by trying to execute something in *totally screwed* memory space/registers.
not counting that talkback fails to display its dialog in some crashes.

anyway, managers know better :)




dvedtiz, per comment #14 do you find it wise to file a bug "talkback may help exploitability" ?
breakpad ftw
have worked as a coder and know that some bugs are difficult to fix. but a classic sql injection and Bug 369188 are clearly COLLECTIVE MANAGEMENT'S RESPONSIBILITY. 

hope that some poor coder is not the scape goat in this bug.
Georgi:  Thanks for the info/help!  It helped me get started on fixing this.  I have made some changes that *should* help here, but wanted to ask you if you could spend some time testing the fix by throwing some other tricks at it...  I think I have the ' and " cases covered, but if there are others, please let me know.  Give the tool a try again.

I'm learning as I go here, so I appreciate your help! Hopefully I'll have all the holes plugged soon. :-)
Scratch my last comment, I'm reverted back to the old code and am investigating the correct way to handle this.  I got some good docs to read and will try to patch things up soon, hopefully by next week.
Jay - Do you need help with the fix ? I can help code review.

This is the only Oracle app we have and the code isn't well owned - so it takes longer than normal to fix even easier issues.  This is not an excuse - just wanted you to know  folks are working on it...
I have made some preliminary changes to the java code to use prepared statements for both the QuickSearch and FastFind tools at http://talkback-public.mozilla.org/search/start.jsp

If folks (Georgi) can test against the latest app and give me some feedback, that will be great!  I just want to make sure all the holes are plugged.

Shiva and Chris Tuft are going to review my code to make sure I did things the right way.  I will wait until they are happy with my changes before marking this bug FIXED.

We should be a lot better now than we were before.
hi jay,

if you are building a single string escaping ' and " probably you should escape the escape character (not ESC, usually \) - this is just a general comment, have not tested talkback yet.
on second thought it is better to whitelist chars instead of escaping

the orignal bug seems fixed.

talkback-public doesn't seem to work as of now - searching for memcpy doesn't produce results in about 15 minutes.
(In reply to comment #26)
> the orignal bug seems fixed.

Yeah, the original testcases look ok now.

> talkback-public doesn't seem to work as of now - searching for memcpy doesn't
> produce results in about 15 minutes.

But although some queries work fine, others are causing the app to hang.  Not sure why, but will continue to investigate. 

looks like the database has regressed since several hours.

tried ' and " in several fields and they worked several hours ago.

now they hang.
Marking FIXED, since the sql injection bug is resolved.  I will log a new bug on the remaining performance issues (things are already a bit better since the last comment).
Status: ASSIGNED → RESOLVED
Closed: 14 years ago13 years ago
Resolution: --- → FIXED
Group: webtools-security
You need to log in before you can comment on or make changes to this bug.