Last Comment Bug 767159 - remove synchronous DNS resolution in nsSOCKSIOLayer.cpp
: remove synchronous DNS resolution in nsSOCKSIOLayer.cpp
Status: RESOLVED FIXED
[Snappy:P1]
:
Product: Core
Classification: Components
Component: Networking: DNS (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla16
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on: CVE-2013-0764
Blocks: 766973
  Show dependency treegraph
 
Reported: 2012-06-21 14:34 PDT by Josh Aas
Modified: 2013-01-10 09:56 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 0 (8.83 KB, patch)
2012-06-26 13:44 PDT, Patrick McManus [:mcmanus]
cbiesinger: review+
Details | Diff | Splinter Review

Description Josh Aas 2012-06-21 14:34:46 PDT
netwerk/socket/nsSOCKSIOLayer.cpp:
219 PRStatus
220 nsSOCKSSocketInfo::ConnectToProxy(PRFileDesc *fd)
[...]
234         rv = dns->Resolve(mProxyHost, 0, getter_AddRefs(mDnsRec));

We probably shouldn't ever make synchronous DNS resolution calls, and we definitely shouldn't do it on the main thread. Removing this stuff is a high priority.
Comment 1 Patrick McManus [:mcmanus] 2012-06-26 13:44:59 PDT
Created attachment 636862 [details] [diff] [review]
patch 0

while I do some more testing lets call this WIP
Comment 2 Patrick McManus [:mcmanus] 2012-06-27 07:20:27 PDT
Comment on attachment 636862 [details] [diff] [review]
patch 0

confirmed behavior before/after patch matches for:

* normal dns based proxy
* ip based proxy
* dns resolution that takes a long time
* invalid host name
* valid hostname, but invalid proxy

Christian, do you have time to look at this?
Comment 3 Christian :Biesinger (don't email me, ping me on IRC) 2012-06-28 14:56:25 PDT
Comment on attachment 636862 [details] [diff] [review]
patch 0

nice :)
Comment 4 Patrick McManus [:mcmanus] 2012-06-28 15:27:11 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/116dcbbd39ba
Comment 5 Ed Morley [:emorley] 2012-06-29 00:46:14 PDT
https://hg.mozilla.org/mozilla-central/rev/116dcbbd39ba

Note You need to log in before you can comment on or make changes to this bug.