Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report taint sink non string #8

Open
wants to merge 10 commits into
base: primitaint-merge
Choose a base branch
from

Conversation

alexbara2000
Copy link

Added the ability to report taint sinks that are not strings. In most cases, even if an object is sent to a sink, it is implicitly transformed into a string before going to the sink. However, there are some cases like messageport sink that send the JSValue itself without implicitly transforming it to a NSString. This PR deals with these cases.

JS_ReportTaintSink(cx, sink, arg, number.taint(), value, true);

}
else if(value.isObject()){
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you want to do here, but this could be fairly spammy. Is this missing a check for taintedness?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree that it could be pretty spammy going over an object recursively to see if it is tainted. However, I don't see a better way of doing it. The JSObject does not hold a flag to say if it is tainted or not. So, to know if a value inside this object is tainted, we have to go over it recursively. On the bright side, this piece of code will not be called often. Most sinks like XML Http request will automatically convert JSObjects to nsACString before calling the report taint sinks.

Under the hood, XML Http request will take an object and do the equivalent of JSON.Stringify before calling the send function. So from a report taint sink perspective, it will see this as a string not an object and the the recursive code will not be called.

This change is to support the new sink that I have added for MessagePort.postMessage. This is one of the only sinks that sends the JSValue itself instead of the nsACString. For this case, we have to look over the object and check to see if it is tainted or not.

That being I am open to suggestions on how to make this code better. Maybe add a flag to JSObject to keep track if it is tainted or not?

@alexbara2000 alexbara2000 marked this pull request as ready for review August 8, 2024 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants