642580 - 500 Internal Server Errors on /query with malformed/unsanitized queries

Status: VERIFIED FIXED

(Socorro :: Webapp, task)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: List of URLs

STR:

1. Either load any of the URLs in the attachment, or enter "https://crash-stats.stage.mozilla.com/query" into the "Target URL" field of PowerFuzzer (http://www.powerfuzzer.com/)
2. Hit Enter or "Scan" in PowerFuzzer

Comment 1

[Robert Helmer [:rhelmer]]

Bug 641879 lets these queries get a little further, the new failures are all along the lines of:

"""
2011-03-17 13:40:46 -07:00 --- error: [5xx Error] File: application/libraries/drivers/Database/Pgsql.php; Line: 74; Message: pg_query(): Query failed: ERROR:  syntax error at or near ")"
LINE 3: ...oduct = 'on')) AND (branches.branch = '2.0') AND () AND  rep...
"""

We should make sure we're doing validation on the user-supplied data, and also ensure that all required fields are present before building the query.

Comment 2

[Robert Helmer [:rhelmer]]

Attachment: check params and query result

* fix previous patch, return value not key
* check all non-free-form queries against acceptable array
* check for reports.product result before building SQL

It looks like we're sanity-checking the contents of user input (Kohana does this) and using db->escape(), so I think we're ok there, this looks like we just don't handle missing or nonsensical query parameters well (alternatively to any of this we could just throw an error, but sanity-checking and falling back to reasonable defaults is a little nicer to users who might tweak the URL).

Comment 3

[Robert Helmer [:rhelmer]]

Committed revision 3025.

Comment 4

[Robert Helmer [:rhelmer]]

This is live on stage now too.

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Post-fix screenshot

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; thanks!