stephenfewer / grinder

Grinder is a system to automate the fuzzing of web browsers and the management of a large number of crashes.
BSD 3-Clause "New" or "Revised" License
415 stars 131 forks source link

Crash display query is incorrect and may produce unreliable results. #24

Closed pyoor closed 10 years ago

pyoor commented 10 years ago

The query used to display crashes improperly groups results and may be unpredictable.

SELECT id, hash_quick, hash_full, verified, node, target, fuzzer, type, time, count, SUM(count) FROM crashes GROUP BY ...

If results are removed from the table (i.e. a hash_quick group is deleted), crashes.php will no longer display crashes and their verification status correctly.

A groupwise max query can be used to solve this: http://jan.kneschke.de/projects/mysql/groupwise-max/

Grouping by MIN(id) will ensure that the earliest appearance of a crash, that with the lowest id value associated with it, is marked as verified when displaying and updating crashes.

stephenfewer commented 10 years ago

Thanks for the heads up, I'll need to read up on this a little before I get a patch ready, cheers :)

pyoor commented 10 years ago

No problem. I've been trying to solve this myself but haven't managed to do so due to the way the query is structured. This is a prerequisite to a patch I'm planning on pushing that will allow you to delete crash hashes from within the web UI.

pyoor commented 10 years ago

I think I've found a fairly clean solution to this though it requires relabeling the verified column: $verified_unknown = 0; $verified_uninteresting = 1; $verified_interesting = 2; $verified_exploitable = 3;

This way, you can issue a sub-query that will return MAX(verified) and match on that result. This will ensure that your display results will always show the result with the most positive rating (i.e. in order of exploitable, interesting, uninteresting, unknown).

I'll submit my commits once I've had a chance to validate the results.

stephenfewer commented 10 years ago

Closing this ticket as is was resolved via commit 6e4de97a67ed2c224fae87e65999538a87eec15e, thanks!