Hi, I want to report a bug to u. This is the first time I try to report a bug. So if anything is wrong, I will sorry about this. The bug at |DFGStrengthReductionPhase|, when it deal ArithMod instruction(https://github.com/WebKit/WebKit/blob/main/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp#L191): case ArithMod: // On Integers // In: ArithMod(ArithMod(x, const1), const2) // Out: Identity(ArithMod(x, const1)) // if const1 <= const2. if (m_node->binaryUseKind() == Int32Use && m_node->child2()->isInt32Constant() && m_node->child1()->op() == ArithMod && m_node->child1()->binaryUseKind() == Int32Use && m_node->child1()->child2()->isInt32Constant() && std::abs(m_node->child1()->child2()->asInt32()) <= std::abs(m_node->child2()->asInt32())) { convertToIdentityOverChild1(); } break; the optimize is so simple, like this: ```c++ let x1 = n % 10; // [+] @a let x2 = x1 % 14; // [+] @b ``` at here, @a we could ensure x1 < 10, then we know 10 < 14, so we think we could remove @b. However, the wrong part is this: `` std::abs(m_node->child1()->child2()->asInt32()) ``` assume |m_node->child1()->child2()->asInt32()| value is |INT_MIN|, |@x: "-2147483648"|, we should got |@y: "2147483648"|, @y > INT_MAX. so interger overflow will happend. ## poc part My poc like this, It could trigger in safari newest version(14.1.2). ``` js let hot_count = 0x1000000; function jit(num){ num |= 0; // [+] without this, num will be (SpecNonBoolInt32 | SpecNonIntAsDouble), which won't got this let x1 = num % -2147483648; let x2 = x1 % 5; // [+] here is emmmmm... if(x2 > 9){ // [+] shold be here... print("[+] magic happend, sir!"); return x2; // [+] I am already calc } return x1 + x2; } for(let i = 0; i < hot_count; i++){ jit(-(i % 10)); } let MAGIC_INT = 10; // [+] if |0x80000000| be thought as a INT, which should be INT_MIN let res = jit(MAGIC_INT); print("[+] the result is: " + res); // [+] should be 0x80000000, error output -2147483648 ``` if u run the poc, you could see x2 is |10|, obviously it should be 0(10 % 5 == 0). ## exploit part I am so sorry, I don't archive exploit part. I try to use this bug to eliminate boundCheck, but Finally I don't find a way. The DFGInterger range analysis part use |executeNode| to determine value's range, but ArithMod aren't be here. I did some source code review, and found seems, if we have a code like this: ```js let x = n % 100; ``` Seems jsc can't aware that x will be: (x > -100 %% x < 100). But the bug seems as this two bug(and another v8's bug), So I decide to report it as a security vulnerablity: - @1: https://googleprojectzero.blogspot.com/2020/09/jitsploitation-one.html - @2: https://bugs.webkit.org/show_bug.cgi?id=229869 @2 seems is found by apple self, seems It is hard to found it by fuzz, I have no idea How can Convert it to a security issue, maybe the bug report know that. But I have not ability to access it. ## another I am sorry for my bad English, sorry....
<rdar://problem/83229740>
The js code has some commit in it... which is wrong, I just changed another bug's poc, and forget delete it. I am so sorry for that part. But the bug is so simple(maybe...), hope it won't confuse u.
By the way, the patch for the bug is simple, add a check like this(https://github.com/WebKit/WebKit/blob/main/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp#L166). if(m_node->child1()->child2()->asInt32() == INT_MIN){ break; // [+] don't optimize it, otherwise it will trigger integer overflow }
hi. nobody care about this?
(In reply to wjllz from comment #4) > hi. nobody care about this? We are looking into it and will respond soon. I apologize for the delay.
(In reply to Brent Fulgham from comment #5) > (In reply to wjllz from comment #4) > > hi. nobody care about this? > > We are looking into it and will respond soon. I apologize for the delay. It's ok, It's my wrong. This is my first time try to report bug to safari, so I don't know I find the right way to report it. So I ask this question(comment 4)... I don't find any useful information about webkit's bounty system... so It make me confusion. Thanks for your reply.
I'm currently investigating this issue. I'm not sure it's really a security issue yet, but will find out soon.
This is not a security issue because there's no way to use the resultant integer to access memory. Any memory access based on the resultant integer thereafter will still do the needed bounds checks.
Thanks for the bug report. Fix coming soon.
Created attachment 439662 [details] proposed patch.
Created attachment 439674 [details] proposed patch.
Created attachment 439675 [details] proposed patch.
Comment on attachment 439675 [details] proposed patch. r=me
Comment on attachment 439675 [details] proposed patch. Thanks for the review. Landing now.
Committed r283300 (242325@main): <https://commits.webkit.org/242325@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 439675 [details].