Closed Bug 477432 Opened 16 years ago Closed 5 years ago

Static analysis to ensure we don't spin the event loop at inopportune times

Categories

(Developer Infrastructure :: Source Code Analysis, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jruderman, Unassigned)

References

Details

(Keywords: sec-want, Whiteboard: [sg:want P3])

I think we've had a fair number of security bugs where the event loop was spun with lots of stuff on the stack, some code on the stack didn't expect some things to be changed, and things blew up as a result.

Examples: bug 465832, bug 459439, maybe bug 345627.

Can this kind of bug be prevented by static analysis, at least on non-Windows platforms?  Maybe with something similar to JS_REQUIRES_STACK?
Possibly, yes. The problem is that you might need to annotate large-ish swaths of code in order for this to be useful. Since most code can spin an event loop (certainly most-any arbitrary XPCOM methods can), we'd probably have to annotate and enforce methods that can't spin an event loop.

Is there a good static definition of "spinning an event loop"?
Whiteboard: [sg:want P3]
I brought this up on dev.platform at one point.

I don't believe it's true that "most code" can spin the event loop. I believe most code cannot. To prove it we probably need to somehow prove that most of the XPCOM interfaces we use internally are not implemented with arbitrary JS.
One way to do that would be to introduce an annotation on interfaces that we do not expect to be implemented with JS, and make XPConnect refuse to support JS implementations of those interfaces.
The sixgill-based static analysis for GC hazards is also a check for "we don't do X during Y" using a call graph. Maybe it could be used for event loop spinning too?
Product: Core → Firefox Build System
Priority: -- → P3
Flags: needinfo?(sledru)
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(sledru)
Resolution: --- → WONTFIX
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.