JS: better support for forms in js/xss-through-dom #4774
Conversation
| result = read | ||
| | | ||
| read.mayHavePropertyName(_) and | ||
| not read.mayHavePropertyName(getADomPropertyName()) |
There was a problem hiding this comment.
With the addition of this rule it seems we now consider all properties of document to be DOM value sources.
Maybe we should just express that in a more direct way.
There was a problem hiding this comment.
Are you suggesting that I put documentRef().getAPropertyRead(any(string s | not s = getADomPropertyName())) into DefaultRange?
I'm also using the forms predicate further down, so I would also like to keep the prop-read inside the forms predicate.
There was a problem hiding this comment.
I'm just saying that all of documentRef().getAPropertyRead() is now a DOM value source, but it's done in a very non-obvious, roundabout way, by unioning three sets:
- all prop reads with a DOM property name
- all prop reads with a non-DOM property name, and
- all property reads with an unknown property name.
If we end up with all of them anyway, I'd just prefer to have documentRef().getAPropertyRead() be listed as an explicit case in DefaultRange. It's fine to keep the forms() predicate.
There was a problem hiding this comment.
documentRef().getAPropertyRead() is now a DOM value source
Hmmm. That turns out not to be the case.
I tried to add documentRef().getAPropertyRead(), but then the tests failed because document.location became a domValueRef.
domValueRef() does not contain documentRef(), so the property-read near the top of DefaultRange' does not refer to document`.
|
Gets a TP/TN for CVE-2018-8035
We must have lost the TP with f23c603.
And this is my attempt at recognizing the source again.