701920 - use prepared statements in dxr to avoid sql injection

Status: RESOLVED FIXED

(Webtools Graveyard :: DXR, defect)

Description

[(dormant account)]

http://dxr.mozilla.org/mozilla/search.cgi?tree=mozilla-central&string=%22NETWORK_DISK_CACHE_OPEN%22

Comment 1

[Christian Dywan]

Attachment: Replace like_escape with placeholders and avoid fetch_all

I propose using placeholders and abandon the custom quoting. Also since I noticed it, I removed fetch_all here and a handful more places so that rows are iterated over and not kept in memory completely.

Comment 2

[Christian Kellner]

Why not use the DB-API’s parameter substitution (http://docs.python.org/library/sqlite3.html#sqlite3.Cursor.execute)? Or am I missing something here?

Comment 3

[Christian Kellner]

Ah, Christian's patch is doing that. I was confused by the name placeholders. Sorry for the noise!

Comment 4

[Christian Dywan]

Attachment: use prepared statements in dxr to avoid sql injection #2

I discovered that I left out the % from the LIKE, the updated patch corrects that.

Comment 5

[(no longer active)]

How does removing the fetchall calls help with SQL injection attacks?

Comment 6

[(no longer active)]

Comment on attachment 575201 [details] [diff] [review]
use prepared statements in dxr to avoid sql injection #2

Clearing the request waiting on the response to my comment...

Comment 7

[Christian Dywan]

(In reply to Ehsan Akhgari [:ehsan] from comment #5)
> How does removing the fetchall calls help with SQL injection attacks?

It doesn't. I just wasn't keen on a tiny second patch with that huge number of unreviewed patches waiting to conflict. Feel free to split it or whatever.

Comment 8

[(no longer active)]

Comment on attachment 575201 [details] [diff] [review]
use prepared statements in dxr to avoid sql injection #2

Nah, not worth splitting...

Comment 9

[Lionel Dricot]

That one is mostly fixed