176114 2017-08-30 11:27:08 -0700 semicolon is being interpreted as an = in the LiteralParser 2017-09-03 13:01:52 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Local Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=176115 InRadar P2 Normal --- 1 oliver saam benjamin bfulgham commit-queue erights fpizlo ggaren gskachkov jfbastien keith_miller ljharb ljokerp mark.lam mathias msaboff oliver rmondello rmorisset saam ticaiolima webkit-bug-importer ysuzuki oldest_to_newest 1343614 0 oliver 2017-08-30 11:27:08 -0700 The following test cases fail x = 0; x;5 assert(x != 5) var y;5 assert(y!=5) etc 1343639 1 saam 2017-08-30 12:19:31 -0700 *** Bug 176115 has been marked as a duplicate of this bug. *** 1343640 2 saam 2017-08-30 12:21:18 -0700 (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 1343651 3 saam 2017-08-30 12:45:35 -0700 I've verified commenting out the literal parser for programs inside Interpreter.cpp removes this bug. 1343654 4 oliver 2017-08-30 12:52:27 -0700 have there been any changes in that code recently? because this seems to be a regression from shipping. Very strange. 1343655 5 saam 2017-08-30 12:56:21 -0700 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. 1343656 6 saam 2017-08-30 12:58:10 -0700 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. 1343657 7 oliver 2017-08-30 13:00:28 -0700 (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. 1343663 8 saam 2017-08-30 13:07:40 -0700 It's not related to: https://bugs.webkit.org/show_bug.cgi?id=156953 1343664 9 oliver 2017-08-30 13:09:28 -0700 (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. 1343665 10 saam 2017-08-30 13:10:31 -0700 (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. 1343668 11 saam 2017-08-30 13:16:00 -0700 Ima work on this. 1343672 12 saam 2017-08-30 13:24:53 -0700 This code looks suspicious inside the lex() function: if (*m_ptr == ';') { token.type = TokSemi; token.end = ++m_ptr; return TokAssign; } 1343674 13 oliver 2017-08-30 13:29:33 -0700 (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 :( 1343675 14 saam 2017-08-30 13:31:44 -0700 Patch forcoming. I'm adding an assertion to next() for this. 1343677 15 319392 saam 2017-08-30 13:45:57 -0700 Created attachment 319392 patch 1343741 16 319392 commit-queue 2017-08-30 15:27:13 -0700 Comment on attachment 319392 patch Clearing flags on attachment: 319392 Committed r221400: <http://trac.webkit.org/changeset/221400> 1343742 17 commit-queue 2017-08-30 15:27:15 -0700 All reviewed patches have been landed. Closing bug. 1343744 18 webkit-bug-importer 2017-08-30 15:28:22 -0700 <rdar://problem/34173912> 1343863 19 ljokerp 2017-08-30 21:43:13 -0700 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 1343875 20 oliver 2017-08-30 22:06:52 -0700 (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. 1343879 21 ljokerp 2017-08-30 22:39:53 -0700 (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. 319392 2017-08-30 13:45:57 -0700 2017-08-30 15:27:13 -0700 patch a-backup.diff text/plain 3821 saam SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyMjEzOTUpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDEzIEBACisyMDE3LTA4LTMwICBTYWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUu Y29tPgorCisgICAgICAgIHNlbWljb2xvbiBpcyBiZWluZyBpbnRlcnByZXRlZCBhcyBhbiA9IGlu IHRoZSBMaXRlcmFsUGFyc2VyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNzYxMTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIHN0cmVzcy9qc29ucC1saXRlcmFsLXBhcnNlci1zZW1pY29sb24taXMtbm90 LWFzc2lnbm1lbnQuanM6IEFkZGVkLgorICAgICAgICAqIHN0cmVzcy9yZXNvdXJjZXMvbGl0ZXJh bC1wYXJzZXItdGVzdC1jYXNlLmpzOiBBZGRlZC4KKwogMjAxNy0wOC0zMCAgT2xla3NhbmRyIFNr YWNoa292ICA8Z3NrYWNoa292QGdtYWlsLmNvbT4KIAogICAgICAgICBbRVNOZXh0XSBBc3luYyBp dGVyYXRpb24gLSBJbXBsZW1lbnQgYXN5bmMgaXRlcmF0aW9uIHN0YXRlbWVudDogZm9yLWF3YWl0 LW9mCkluZGV4OiBKU1Rlc3RzL3N0cmVzcy9qc29ucC1saXRlcmFsLXBhcnNlci1zZW1pY29sb24t aXMtbm90LWFzc2lnbm1lbnQuanMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNUZXN0cy9zdHJlc3MvanNvbnAt bGl0ZXJhbC1wYXJzZXItc2VtaWNvbG9uLWlzLW5vdC1hc3NpZ25tZW50LmpzCShub25leGlzdGVu dCkKKysrIEpTVGVzdHMvc3RyZXNzL2pzb25wLWxpdGVyYWwtcGFyc2VyLXNlbWljb2xvbi1pcy1u b3QtYXNzaWdubWVudC5qcwkod29ya2luZyBjb3B5KQpAQCAtMCwwICsxLDQgQEAKK3ggPSB1bmRl ZmluZWQ7Citsb2FkKCIuL3Jlc291cmNlcy9saXRlcmFsLXBhcnNlci10ZXN0LWNhc2UuanMiKTsK K2lmICh4ICE9PSB1bmRlZmluZWQpCisgICAgdGhyb3cgbmV3IEVycm9yKCJCYWQgcmVzdWx0Iik7 CkluZGV4OiBKU1Rlc3RzL3N0cmVzcy9yZXNvdXJjZXMvbGl0ZXJhbC1wYXJzZXItdGVzdC1jYXNl LmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvc3RyZXNzL3Jlc291cmNlcy9saXRlcmFsLXBhcnNl ci10ZXN0LWNhc2UuanMJKG5vbmV4aXN0ZW50KQorKysgSlNUZXN0cy9zdHJlc3MvcmVzb3VyY2Vz L2xpdGVyYWwtcGFyc2VyLXRlc3QtY2FzZS5qcwkod29ya2luZyBjb3B5KQpAQCAtMCwwICsxIEBA Cit4OzEyMzQKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjIxMzk1KQor KysgU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDIyIEBACisyMDE3LTA4LTMwICBTYWFtIEJhcmF0aSAgPHNiYXJhdGlAYXBwbGUuY29tPgor CisgICAgICAgIHNlbWljb2xvbiBpcyBiZWluZyBpbnRlcnByZXRlZCBhcyBhbiA9IGluIHRoZSBM aXRlcmFsUGFyc2VyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xNzYxMTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBXaGVuIGxleGluZyBhIHNlbWljb2xvbiBpbiB0aGUgTGl0ZXJhbFBhcnNlciwgd2Ugd2Vy ZSBwcm9wZXJseQorICAgICAgICBzZXR0aW5nIHRoZSBUb2tlblR5cGUgb24gdGhlIGN1cnJlbnQg dG9rZW4sIGhvd2V2ZXIsIHdlIHdlcmUKKyAgICAgICAgKnJldHVybmluZyogdGhlIHdyb25nIFRv a2VuVHlwZS4gVGhlIGxleCBmdW5jdGlvbiBib3RoIHJldHVybnMKKyAgICAgICAgdGhlIFRva2Vu VHlwZSBhbmQgc2V0cyBpdCBvbiB0aGUgY3VycmVudCB0b2tlbi4gU2VtaWNvbG9uIHdhcworICAg ICAgICBzZXR0aW5nIHRoZSBUb2tlblR5cGUgdG8gc2VtaWNvbG9uLCBidXQgcmV0dXJuaW5nIHRo ZSBUb2tlblR5cGUKKyAgICAgICAgZm9yICc9Jy4gVGhpcyBjYXVzZWQgcHJvZ3JhbXMgbGlrZSBg eDsxMjNgIHRvIGJlIGludGVycHJldGVkIGFzCisgICAgICAgIGB4PTEyM2AuCisKKyAgICAgICAg KiBydW50aW1lL0xpdGVyYWxQYXJzZXIuY3BwOgorICAgICAgICAoSlNDOjpMaXRlcmFsUGFyc2Vy PENoYXJUeXBlPjo6TGV4ZXI6OmxleCk6CisgICAgICAgIChKU0M6OkxpdGVyYWxQYXJzZXI8Q2hh clR5cGU+OjpMZXhlcjo6bmV4dCk6CisKIDIwMTctMDgtMjIgIEZpbGlwIFBpemxvICA8ZnBpemxv QGFwcGxlLmNvbT4KIAogICAgICAgICBTdHJpbmdzIG5lZWQgdG8gYmUgaW4gc29tZSBraW5kIG9m IGdpZ2FjYWdlCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9MaXRlcmFsUGFy c2VyLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9MaXRl cmFsUGFyc2VyLmNwcAkocmV2aXNpb24gMjIxMzk1KQorKysgU291cmNlL0phdmFTY3JpcHRDb3Jl L3J1bnRpbWUvTGl0ZXJhbFBhcnNlci5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI3Miw3ICsyNzIs NyBAQCB0ZW1wbGF0ZSA8UGFyc2VyTW9kZSBtb2RlPiBUb2tlblR5cGUgTGl0CiAgICAgICAgIGlm ICgqbV9wdHIgPT0gJzsnKSB7CiAgICAgICAgICAgICB0b2tlbi50eXBlID0gVG9rU2VtaTsKICAg ICAgICAgICAgIHRva2VuLmVuZCA9ICsrbV9wdHI7Ci0gICAgICAgICAgICByZXR1cm4gVG9rQXNz aWduOworICAgICAgICAgICAgcmV0dXJuIFRva1NlbWk7CiAgICAgICAgIH0KICAgICAgICAgaWYg KGlzQVNDSUlBbHBoYSgqbV9wdHIpIHx8ICptX3B0ciA9PSAnXycgfHwgKm1fcHRyID09ICckJykK ICAgICAgICAgICAgIHJldHVybiBsZXhJZGVudGlmaWVyKHRva2VuKTsKQEAgLTMxNywxMSArMzE3 LDE1IEBAIEFMV0FZU19JTkxJTkUgVG9rZW5UeXBlIExpdGVyYWxQYXJzZXI8VUMKIHRlbXBsYXRl IDx0eXBlbmFtZSBDaGFyVHlwZT4KIFRva2VuVHlwZSBMaXRlcmFsUGFyc2VyPENoYXJUeXBlPjo6 TGV4ZXI6Om5leHQoKQogeworICAgIFRva2VuVHlwZSByZXN1bHQ7CiAgICAgaWYgKG1fbW9kZSA9 PSBOb25TdHJpY3RKU09OKQotICAgICAgICByZXR1cm4gbGV4PE5vblN0cmljdEpTT04+KG1fY3Vy cmVudFRva2VuKTsKLSAgICBpZiAobV9tb2RlID09IEpTT05QKQotICAgICAgICByZXR1cm4gbGV4 PEpTT05QPihtX2N1cnJlbnRUb2tlbik7Ci0gICAgcmV0dXJuIGxleDxTdHJpY3RKU09OPihtX2N1 cnJlbnRUb2tlbik7CisgICAgICAgIHJlc3VsdCA9IGxleDxOb25TdHJpY3RKU09OPihtX2N1cnJl bnRUb2tlbik7CisgICAgZWxzZSBpZiAobV9tb2RlID09IEpTT05QKQorICAgICAgICByZXN1bHQg PSBsZXg8SlNPTlA+KG1fY3VycmVudFRva2VuKTsKKyAgICBlbHNlCisgICAgICAgIHJlc3VsdCA9 IGxleDxTdHJpY3RKU09OPihtX2N1cnJlbnRUb2tlbik7CisgICAgQVNTRVJUKG1fY3VycmVudFRv a2VuLnR5cGUgPT0gcmVzdWx0KTsKKyAgICByZXR1cm4gcmVzdWx0OwogfQogCiB0ZW1wbGF0ZSA8 Pgo=