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

VERIFIED FIXED in 1.7.7

Status

Socorro
Webapp
--
major
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: stephend, Unassigned)

Tracking

Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fuzzer], URL)

Attachments

(3 attachments)

(Reporter)

Description

7 years ago
Created attachment 520013 [details]
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
Flags: in-testsuite?
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.
Assignee: nobody → rhelmer
Status: NEW → ASSIGNED
Created attachment 520054 [details] [diff] [review]
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).
Attachment #520054 - Flags: review?(ryan)
Attachment #520054 - Flags: review?(ryan) → review+
Committed revision 3025.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
This is live on stage now too.
(Reporter)

Comment 5

7 years ago
Created attachment 522838 [details]
Post-fix screenshot
(Reporter)

Comment 6

7 years ago
Verified FIXED; thanks!
Status: RESOLVED → VERIFIED
(Assignee)

Updated

6 years ago
Component: Socorro → General
Product: Webtools → Socorro
(Reporter)

Updated

6 years ago
Assignee: rhelmer → nobody
Component: General → Webapp
QA Contact: socorro → webapp
(Reporter)

Updated

6 years ago
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.