Open Bug 1736082 Opened 3 years ago Updated 1 year ago

Use hazard analysis for checking and completing MOZ_CAN_RUN_SCRIPT annotations

Categories

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

enhancement

Tracking

(Not tracked)

People

(Reporter: sfink, Unassigned)

References

(Blocks 1 open bug)

Details

As mentioned in bug 1380423 comment 24, we could use the hazard analysis framework to spread MOZ_CAN_RUN_SCRIPT throughout the tree and be more confident that it is correct and complete.

Could you please detail about this?

Flags: needinfo?(sphink)
Severity: -- → S3
Type: task → enhancement
Priority: -- → P3

There are multiple possibilities here. They're all answers to the question "how could the 'can run script' analysis be improved with a global analysis?"

One option: The JS engine has limited entry points for running script. If those (actually, there might only be one, js::RunScript?) were annotated, say with MOZ_RUN_SCRIPT, then the entire callgraph could be labeled with can-reach-MOZ_RUN_SCRIPT booleans. We could then report anything not labeled with MOZ_CAN_RUN_SCRIPT but can-reach-MOZ_RUN_SCRIPT, probably filtered by some coarse-grained rules (eg don't report anything in the js:: or JS:: namespaces, or in the JS_* pseudo-namespace). This could either eliminate MOZ_CAN_RUN_SCRIPT_BOUNDARY, or we could still use that to demarcate regions that don't require annotation and filter those out (in this case, the global analysis would largely be for ensuring that all the boundaries are correct).

More specifically, with the current setup if you had:

A -> B -> C -> js::RunScript
X -> C -> js::RunScript

where B is MOZ_CAN_RUN_SCRIPT_BOUNDARY, A is MOZ_CAN_RUN_SCRIPT, and neither C nor X is annotated. Then my current understanding is that no error would be reported. But X might be an important function vulnerable to the problem MOZ_CAN_RUN_SCRIPT is intended to solve. The global analysis, using coarse filtering rules, would report that X should be MOZ_CAN_RUN_SCRIPT.

It would also say that C should be as well, but perhaps the noise could be reduced by noticing that C is sometimes behind a boundary, so it could report that X should be annotated and there should probably be a boundary somewhere on the X -> C path. That may be overkill. You could run once and have it report all of the leaves, and then manually go through and figure out what should be annotated how, then rerun.

Another option would be to eliminate all of these annotations, generate them automatically, and then recompile everything with the existing plugin while looking up annotations in the generated set. But I doubt that's workable or even that valuable.

Something that would need to be worked out is how conservative the global scan should be. Should calls through function pointers be assumed to be able to run script unless annotated otherwise? The existing analysis already takes a position on this (no), but if it were backed up by a global analysis then it might be feasible to be more complete. (I kinda doubt it, though.)

Finally, the global analysis could close the bug 1535523 hole too.

Flags: needinfo?(sphink)
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.