Closed
Bug 1167685
Opened 9 years ago
Closed 9 years ago
Potentially-unsafe code in VirtualSocketServer::Evaluate
Categories
(Core :: WebRTC, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: q1, Unassigned)
Details
(Keywords: sec-audit, sec-other)
User Agent: Mozilla/5.0 (Windows; rv:***) Gecko/20100101 Firefox/**.* Build ID: 20150305021524 Steps to reproduce: VirtualSocketServer::Evaluate (media\webrtc\trunk\webrtc\base\virtualsocketserver.cc) might be able to reference a nonexistent element of a std::vector: 1045: double VirtualSocketServer::Evaluate(Function* f, double x) { 1046: Function::iterator iter = 1047: std::lower_bound(f->begin(), f->end(), x, FunctionDomainCmp()); 1048: if (iter == f->begin()) { 1049: return (*f)[0].second; 1050: } else if (iter == f->end()) { 1051: ASSERT(f->size() >= 1); 1052: return (*f)[f->size() - 1].second; 1053: } else if (iter->first == x) { ... The problem occurs if an empty vector *f is possible. In this case, lines 1046-47 set iter==f->end(), which also == f->begin() (because f isa std::vector<Point> *). Then line 1049 references the nonexistent element 0 and returns its .second field, possibly causing incorrect and/or insecure operation down the line. I do not know whether it is possible for *f to be empty when Evaluate is called.
Updated•9 years ago
|
Comment 1•9 years ago
|
||
I'd suggest reporting this to upstream. VirtualSocketServer appears to be used only in unit tests in upstream code. Currently we don't build these unit tests (and we don't build virtualsocketserver.cc) and it's not in any released Firefox executables. You can report it at https://code.google.com/p/webrtc/issues/list. Our code is based on the webrtc 40 stable branch. Likely we should close this bug
Flags: needinfo?(rjesup)
Keywords: sec-other
Comment 2•9 years ago
|
||
This is in code we don't compile or use
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
I suggest making this code uncompilable with a #error directive, so that it gets proper security review if someone decides to use it in the future.
Comment 4•9 years ago
|
||
(In reply to q1 from comment #3) > I suggest making this code uncompilable with a #error directive, so that it > gets proper security review if someone decides to use it in the future. That means we have to make and carry more patches to upstream code (and merge them). And extending that, any ifdefed code we don't compile-configure in would need #errors.... Far better is to just file the bug against the actual source, the upstream project, and if you like add a comment to the moz.build file referencing it.
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•