The following test cases fail x = 0; x;5 assert(x != 5) var y;5 assert(y!=5) etc
*** Bug 176115 has been marked as a duplicate of this bug. ***
(In reply to Oliver Hunt from comment #0) > The following test cases fail > > x = 0; > x;5 > assert(x != 5) > > var y;5 > assert(y!=5) > > etc This is not right. You need code like this: // test.js x=undefined; load("test2.js"); print(x); // prints 123 // test2.js x;123 It appears to be related to the literal parser. If you run the above example with JSC_dumpGeneratedBytecodes=1, we don't even generate bytecode for the second program in test2.js
I've verified commenting out the literal parser for programs inside Interpreter.cpp removes this bug.
have there been any changes in that code recently? because this seems to be a regression from shipping. Very strange.
I have a vague memory of Yusuke doing some work there. It's really bizarre to me that the literal parser is causing a variable assignment. Is that expected behavior? I don't even really understand what the literal parser is for, I always though it was a way to quickly create objects that are in JSON format.
Maybe it was this bug? https://bugs.webkit.org/show_bug.cgi?id=156953 I believe this is the last change there that isn't related to basic refactoring.
(In reply to Saam Barati from comment #5) > I have a vague memory of Yusuke doing some work there. It's really bizarre > to me that the literal parser is causing a variable assignment. Is that > expected behavior? > > I don't even really understand what the literal parser is for, I always > though it was a way to quickly create objects that are in JSON format. LiteralParser is used for JSON and for JSONP. To do JSONP quickly we have magic logic to handle (a(.b)*=json | a(.b)*\(json\))+ which we parse and call directly to avoid jumping through the interpreter. We need this because people still insist on sending megs of data as executable script. because reasons.
It's not related to: https://bugs.webkit.org/show_bug.cgi?id=156953
(In reply to Saam Barati from comment #6) > Maybe it was this bug? > https://bugs.webkit.org/show_bug.cgi?id=156953 > > I believe this is the last change there that isn't related to basic > refactoring. I suspect change would only occur in the jsonp handling. Which used to be in interpreter.cpp but I don't have a webkit checkout handy.
(In reply to Oliver Hunt from comment #4) > have there been any changes in that code recently? because this seems to be > a regression from shipping. Very strange. I can reproduce it in a shipping macOS. So if it's a regression, it is from further back than the last macOS.
Ima work on this.
This code looks suspicious inside the lex() function: if (*m_ptr == ';') { token.type = TokSemi; token.end = ++m_ptr; return TokAssign; }
(In reply to Saam Barati from comment #12) > This code looks suspicious inside the lex() function: > > > if (*m_ptr == ';') { > token.type = TokSemi; > token.end = ++m_ptr; > return TokAssign; > } that does indeed seem questionable. I'm so sad I wrote it :(
Patch forcoming. I'm adding an assertion to next() for this.
Created attachment 319392 [details] patch
Comment on attachment 319392 [details] patch Clearing flags on attachment: 319392 Committed r221400: <http://trac.webkit.org/changeset/221400>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34173912>
It was a nice XSS victor for me. And it's a just little mistake not even a bug. So, thx for Ur mistake & don't write more like "OMG I forget change the return when I paste codes". Have nice day! XD
(In reply to LiJunPan(evil7) from comment #19) > It was a nice XSS victor for me. And it's a just little mistake not even a > bug. > So, thx for Ur mistake & don't write more like "OMG I forget change the > return when I paste codes". > Have nice day! XD The XSS vector is mercifully limited as it requires the right hand side of the assignment to be side effect free (it's not arbitrary expressions, at least it shouldn't be), basically the right hand side has to be a slightly more accepting version of JSON: "s and 's are are allowed for string literals, and a few other minor differences in that vein.
(In reply to Oliver Hunt from comment #20) > (In reply to LiJunPan(evil7) from comment #19) > > It was a nice XSS victor for me. And it's a just little mistake not even a > > bug. > > So, thx for Ur mistake & don't write more like "OMG I forget change the > > return when I paste codes". > > Have nice day! XD > > The XSS vector is mercifully limited as it requires the right hand side of > the assignment to be side effect free (it's not arbitrary expressions, at > least it shouldn't be), basically the right hand side has to be a slightly > more accepting version of JSON: "s and 's are are allowed for string > literals, and a few other minor differences in that vein. Yep.I just found it from a CTF-game at last week. That's a strangely POC but OMG it's worked. SO I tryed to input some others like "foo;test;" or "var test;'string'" wanna make it working but NOT,It ONLY worked with "location;'(String:)str'" and ONLY at macOS. So I realized it's hardly to make a XSS in web framework. Just maybe a little mistake in the sourses of Webkit.