Closed
Bug 1213400
Opened 9 years ago
Closed 9 years ago
Switching list update response for initial download response causes crash
Categories
(Toolkit :: Safe Browsing, defect)
Tracking
()
VERIFIED
FIXED
mozilla45
People
(Reporter: mwobensmith, Assigned: francois)
References
()
Details
Attachments
(3 files)
5.49 KB,
application/zip
|
Details | |
682 bytes,
patch
|
gcp
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
79.39 KB,
text/plain
|
Details |
1. Unzip sample files. 2. Launch pyserver13000.py 3. Launch pyserver13001.py 4. Change Nightly Fx44 prefs for shavar list URL to "localhost:13000" 5. Change "nextupdatetime" pref to 1. 6. Relaunch Nightly via command line, using environment variables from here for debugging: https://wiki.mozilla.org/Phishing_Protection#QA Result: Crash - see URL NOTE: The fun part here is that although the Python scripts above indicate connections on ports 13000 and 13001 respectively, I simply switched them for this test. What then happens is that the initial query for table names, update interval etc. instead gets an update list response.
Reporter | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Here's the reason for that crash: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffca6ff700 (LWP 367)] mozilla::safebrowsing::ProtocolParser::ProcessChunkControl (this=0x7fffca7fb320, aLine=...) at /home/francois/devel/mozilla-central/toolkit/components/url-classifier/ProtocolParser.cpp:199 199 if (!mTableUpdate || mTableUpdate->TableName().IsEmpty()) { (gdb) p mTableUpdate $1 = (mozilla::safebrowsing::TableUpdate *) 0xe4e4e4e4e4e4e4e4 De-referencing an uninitialized pointer.
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8693145 -
Flags: review?(gpascutto)
Updated•9 years ago
|
Attachment #8693145 -
Flags: review?(gpascutto) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8693145 [details] [diff] [review] bug1213400.patch Approval Request Comment [Feature/regressing bug #]: crash on invalid responses from safe browsing / tracking protection servers [User impact if declined]: Could cause a browser crash if the trusted endpoints we use malfunction for some reason (which prevents firefox from working even in safe mode until the server is fixed). [Describe test coverage new/current, TreeHerder]: manual testing as per Matt's steps in this bug [Risks and why]: Prior to this, our null point checks were kind of pointless since the pointer was non-null but pointing to uninitialized memory. So theoretically, this could surface some bugs where we were using an uninitialized pointer without crashing, but it seems unlikely. [String/UUID change made/needed]: none
Attachment #8693145 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to François Marier [:francois] from comment #5) > [User impact if declined]: Could cause a browser crash if the trusted > endpoints we use malfunction for some reason (which prevents firefox from > working even in safe mode until the server is fixed). Correction: safe-mode is not affected by this crash since all of Safe Browsing is disabled in safe-mode (see bug 1134954).
Comment 7•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4d5314a40363
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Matt, could you please verify this issue is fixed as expected on latest Nightly? Thanks!
Flags: needinfo?(mwobensmith)
Comment on attachment 8693145 [details] [diff] [review] bug1213400.patch Crash fixes are good, Aurora44+
Attachment #8693145 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/3c145973b6cb
Reporter | ||
Comment 11•9 years ago
|
||
Nightly 45 looks good. However, I'm still getting a crash on Aurora 44. See attached log. Let me know if you'd like me to do investigate more.
Flags: needinfo?(mwobensmith)
Reporter | ||
Comment 12•9 years ago
|
||
OK, apologies - checkin didn't happen until this morning, so perhaps I jumped the gun. I'll wait until we have a build to try it on Aurora 44. Sorry about that.
Comment 13•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/3c145973b6cb
status-b2g-v2.5:
--- → fixed
Reporter | ||
Comment 14•9 years ago
|
||
Verified on latest builds of both Nightly and Aurora.
(In reply to Matt Wobensmith [:mwobensmith][:matt] from comment #14) > Verified on latest builds of both Nightly and Aurora. This is great. Thank you!
You need to log in
before you can comment on or make changes to this bug.
Description
•