RuleData::_position does not check for overflow
Reported by
bobbyhol...@gmail.com,
Jun 12 2017
|
|||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0 Steps to reproduce the problem: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/RuleSet.h?l=128&rcl=e5d750bac91cfa4133996e8edca695c95d391bba What is the expected behavior? What went wrong? Hi, Mozilla platform developer here. We were discussing Blink's 18-bit source order packing today (linked above), and the consensus was that it's small enough to be hit on realistic web pages. And if that happens, Blink will currently trigger compiler-dependent undefined behavior (overflow when setting packed struct fields is not defined). We ended up settling on 24 bits for our representation. This is probably harder for Blink to do given its usage of the other bits for flags, and the discussion of the exact precision probably needs to happen in spec-land. Regardless, Blink should add an overflow check to avoid exposing UB to web pages. Did this work before? N/A Chrome version: 58.0.3029.110 (Official Build) (64-bit) Channel: n/a OS Version: OS X 10.12 Flash Version: Shockwave Flash 25.0 r0
,
Jun 12 2017
,
Jun 12 2017
,
Jun 14 2017
,
Jun 14 2017
,
Oct 9 2017
,
Dec 6 2017
,
Apr 18 2018
This doesn't seem like a "Feature". It's just going to produce incorrect, possibly-random behavior for some CSS input. Not clear whether there are any security implications there, but this isn't a feature request.
,
Oct 23
,
Oct 23
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b000aed5874a1886a1c31d5c3f02ec13cbef6f2 commit 7b000aed5874a1886a1c31d5c3f02ec13cbef6f2 Author: Anders Hartvoll Ruud <andruud@chromium.org> Date: Thu Oct 25 15:35:11 2018 Check RuleData::position_ for overflow. Exceeding 18 bits is apparently entirely possible according to bug report, so ensure that we gracefully handle this situation. Also added bonus tests for the existing similar case for selector_index. R=futhark@chromium.org Bug: 732502 Change-Id: I1a831a677ab82998c1c0fd925558e3dec1da36d4 Reviewed-on: https://chromium-review.googlesource.com/c/1296503 Commit-Queue: Anders Ruud <andruud@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#602717} [modify] https://crrev.com/7b000aed5874a1886a1c31d5c3f02ec13cbef6f2/third_party/blink/renderer/core/css/rule_feature_set_test.cc [modify] https://crrev.com/7b000aed5874a1886a1c31d5c3f02ec13cbef6f2/third_party/blink/renderer/core/css/rule_set.cc [modify] https://crrev.com/7b000aed5874a1886a1c31d5c3f02ec13cbef6f2/third_party/blink/renderer/core/css/rule_set.h [modify] https://crrev.com/7b000aed5874a1886a1c31d5c3f02ec13cbef6f2/third_party/blink/renderer/core/css/rule_set_test.cc
,
Oct 29
,
Oct 29
Issue 892544 has been merged into this issue. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by bobbyhol...@gmail.com
, Jun 12 2017