Js math taint propagation #3
Conversation
… to make Math library taint aware
|
First of all: Thanks for contributing to foxhound! I had a cursory look at the changes, and they look fine. What's slightly unclear to me is the intention behind these changes. Could you provide some examples of JavaScript code that does not propagate taints if one uses the primitaint-merge HEAD vs. your changes? |
|
Yes of course, I will provide a bit more information about the changes. Javascript has a builtin library called Math which gives users access to math functions like Math.round(x). Since this library is builtin, it needs to be defined in the JS engine which in the case of Firefox it can be found in The issue was that this builtin library was not taint aware meaning that any JS code that uses it would loose it's taint. The simplest example is if you have the following statement This pull request solves this issue by making the whole math library taint aware. Math library is where you can find all the methods that are part of the math library which I have made taint aware. I can also give a more low level explanation of the changes I made if needed. |
|
Ah, interesting, thank you! That makes a ton of sense :) I wonder how this might have impacted the fp-tracer study, as max/min/round seem fairly common operations heh. |
js/src/jsmath.cpp
Outdated
| NumberObject *taintedResult = NumberObject::create(cx, 0); | ||
| bool isTainted = false; | ||
|
|
||
| if(isTaintedNumber(lhs)){ |
There was a problem hiding this comment.
Another thing I noticed - I think a lot of this code can be replaced by a call to JS::getAnyNumberTaint in jstaint.cpp.
Have a look at e.g. the AddOperation in Interpreter-inl.h for an example how this is done.
I have recently (last week!) updated the logic for combining two taint flows together so that in principle both parents in the taint flow are saved. If you use the getAnyNumberTaint call then all of this will be taken in account automatically.
There was a problem hiding this comment.
PS - you might need to update your branch to the HEAD of primitaint-merge to get the latest changes.
In this branch properties of WebIDL files can be marked as taint sources with an attribute. This is super nice, but if one wants to disable one of them one has to note down their names during a build. This is pretty cumbersome. This commit tries to resolve this issue, as it simply adds every IDL based source to the properties defaults.
alexbara2000
force-pushed
the
JSMathTaint
branch
from
f44a0c9
to
4806180
Compare
Added taint propagation for the JSMath builtin library.
Added tests for the JSMath builtin library propagation.