Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1308053002:
blink: prevent bitarray overflow (Closed)
|
Created:
5 years, 4 months ago by hendrikw Modified:
5 years, 4 months ago CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, Eric Willigers, rjwright, shans Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
Descriptionblink: prevent bitarray overflow
While attempting to add a new css property, I ran into
ASSERT_WITH_SECURITY_IMPLICATION(index < arraySize) while calling
CSSAnimations::calculateTransitionUpdate. It looks like we normally don't
include CSSPropertyInvalid, and use firstCSSProperty to offset the index.
I've done the same here.
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201080
Patch Set 1 #
Total comments: 1
Messages
Total messages: 20 (9 generated)
hendrikw@chromium.org changed reviewers: + dstockwell@chromium.org
PTAL, thanks!
The CQ bit was checked by esprehn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308053002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308053002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by esprehn@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308053002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308053002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
alancutter@chromium.org changed reviewers: + alancutter@chromium.org
lgtm with nit. https://codereview.chromium.org/1308053002/diff/1/Source/core/animation/css/C... File Source/core/animation/css/CSSAnimations.cpp (right): https://codereview.chromium.org/1308053002/diff/1/Source/core/animation/css/C... Source/core/animation/css/CSSAnimations.cpp:561: listedProperties.set(id - firstCSSProperty); No need to subtract firstCSSProperty when you have the assert. I think it's simpler if we stick with storing and reading regular CSSPropertyIDs.
> https://codereview.chromium.org/1308053002/diff/1/Source/core/animation/css/C... > Source/core/animation/css/CSSAnimations.cpp:561: listedProperties.set(id - firstCSSProperty); > No need to subtract firstCSSProperty when you have the assert. I think it's simpler if we stick with storing and reading regular CSSPropertyIDs. Sorry I'm wrong, I see the subtraction is necessary now.
The CQ bit was checked by hendrikw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308053002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308053002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hendrikw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1308053002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1308053002/1
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=201080 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
