Assigning to concern owner from the CL,
https://chromium.googlesource.com/chromium/src/+log/0f2c9b7550bb338b5581255003de08e71c913720..abb1ea76a4800b1c6dd3e8c12aaf5c1dc9541f1a?pretty=fuller

Suspecting Commit# 1465f844b54478a027711e86011a19214d8de24d
Suspecting Review URL# https://codereview.chromium.org/1814563002

@bsep -- Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Note: Find it did not provide any results.

Thank You.

No, my change is in the math library that detected the overflow.

Reassigning to danakj@ because she was the last to touch HTMLCanvasElement which looks like the most relevant part of the stack. CC'ing junov@ who's the other obvious owner for this. Sorry if this isn't relevant to you.

0x7f3348684771 in blink::HTMLCanvasElement::updateExternallyAllocatedMemory() is overflowing in this test case. Something needs to be clamped, or creation of the canvas should fail or something.

Ccing owners of base/numerics/

By looking at the crash report, here is few lines of code to demonstrate what happened:

CheckedNumeric<long> a = 25769801688;
int b = 2147483474;
a *= b;

Integer overflow happens on line "a *= b". 

The crash report points to line 226 in base/numerics/safe_math_impl.h:
https://cs.chromium.org/chromium/src/base/numerics/safe_math_impl.h?q=safe_math_impl.h&sq=package:chromium&dr=CSs&l=226

which does this: return static_cast<T>(x * y).

I think since x is already long, it results in undefined behavior.

Do we want to add a line to check "validity" before line 226? I am not sure if that is the best solution or not, because it introduces an additional branch.

Thoughts?

Thanks for the research into where it's going wrong. Assigning to jschuh who wrote the safe numerics stuff.

Nah, what happens is we detect that overflow will happen, and then perform the operation anyhow, and then don't use the result since we flagged it as invalid.  I think we just want to suppress this one since the API enforces safe usage.

Alternately, we could check for invalid and leave it unchanged.

Actually, for the signed case I'm paranoid that the optimizer might go so far as to remove your checks since the multiplication will be performed no matter what, hence overflow can't occur, hence the checks can be removed because they cant fire unless overflow.

Suppressing sounds ok, as that doesn't incur perf impact and the safe numerics code is pretty stable.

I'm not aware of any optimizers using such strong definitions of undefined behavior as tsepez@ suggested. Rather, so far they seem to remove only paths that are directly contingent on the undefined behavior.

That stated, I have a half finished CL that clears up all the undefined behavior, because I wanted to make it clean. So, I'll just assign this to me and use it as an excuse to perf-crastinate.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/749c7f7733ccb09a60eb531f76551f7d709e1bda

commit 749c7f7733ccb09a60eb531f76551f7d709e1bda
Author: jschuh <jschuh@chromium.org>
Date: Wed Sep 07 16:22:26 2016

Fix undefined behavior in base/numerics

BUG= 643585 
R=tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2318713004
Cr-Commit-Position: refs/heads/master@{#416954}

[modify] https://crrev.com/749c7f7733ccb09a60eb531f76551f7d709e1bda/base/numerics/safe_math_impl.h
[modify] https://crrev.com/749c7f7733ccb09a60eb531f76551f7d709e1bda/base/numerics/safe_numerics_unittest.cc

I just landed a very simple patch that eliminates the undefined behavior, without the larger refactoring that it had been bundled into. So, I'm closing this as fixed, but I plan on circling back for some more cleanup in the near future.

ClusterFuzz has detected this issue as fixed in range 416947:416966.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5900439017226240

Fuzzer: inferno_twister
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  _ZN4base8internal10CheckedMulIlEENSt3__19enable_ifIXaaaasr3std14numeric_limitsIT
  _ZN4base8internalmlIlEENS0_14CheckedNumericINS0_19ArithmeticPromotionIT_S4_Xqugt
  _ZN4base8internalmlIilEENS0_14CheckedNumericINS0_19ArithmeticPromotionIT_T0_Xqug
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=382751:382786
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=416947:416966

Minimized Testcase (13.08 Kb): https://cluster-fuzz.appspot.com/download/AMIfv97HQDtoFbzz2ZZgUy8kLDG1sV3bBkwD-2dmhuVvEETtibaBfcJo1RB5HIe5kMn7hCOuKwnjCnVfaQUfe9Ub1aZd0jVt4wrjy6oipDYltSDoNtay_Wq8jXRGcueIS8oVuhp7nqTN44JnX9pwwozxSPPdIw71oA?testcase_id=5900439017226240

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot