Foxhound: Add taint propagation for HashChangeEvent.oldURL and newURL#363
Foxhound: Add taint propagation for HashChangeEvent.oldURL and newURL#363Stouelh wants to merge 1 commit into
Conversation
Signed-off-by: Stouelh <124311326+Stouelh@users.noreply.github.com>
|
|
|
The changes look fine to me from a code point of view. As you pointed out, mochitests are required, but I have an additional request. Please add the sources to the configuration all.js. We probably should do some CI checking that this is synced with the sources/sinks we have, i have opened an issue for that #366. :) My concern with this change is that we have no way to see whether the hash is actually tainted. For example, if I do: location.hash = "baz";The event would fire and report this as tainted, even if it's just a static assignment. I think merging this is fine if you set the default state to false in all.js and manually enable it in your setup. I think all automation frameworks allow you to override Firefox preferences. Would that work for you? I'd prefer if we'd propagate the taints automatically here, but I think it gets lost by assigning it to an NsIURI. Making that conversion taint-aware might do the trick, long-term, as I think this should, in theory, be propagated automatically. But that is a much more involved change, so I'd be happy to merge this as a temporary fix until we look into why it isn't being propagated properly. |
Fixes #323
When a hashchange event fires, the oldURL and newURL properties on the
HashChangeEvent were not being marked as taint sources. This means that
if tainted data (e.g. from location.hash or location.href) triggered a
hash change, Foxhound would lose track of the taint flow through those
event properties.
This fix adds MarkTaintSource calls for both init.mOldURL and init.mNewURL
in FireHashchange() in nsGlobalWindowInner.cpp, following the same pattern
used for location.hash and other existing taint sources.
Note: A mochitest is still needed to verify the fix. Opening as draft for
early feedback.
Signed-off-by: Soliman Touelh stouelh@umich.edu