Page MenuHomePhabricator

Merge #14734: fix an undefined behavior in uint::SetHex
ClosedPublic

Authored by jasonbcox on Jul 7 2020, 18:48.

Details

Reviewers
majcosta
Group Reviewers
Restricted Project
Commits
rABC63a5cddd312d: Merge #14734: fix an undefined behavior in uint::SetHex
Summary

0f459d868d85053f1cc066ea9099793f88cbd655 fix an undefined behavior in uint::SetHex (Kaz Wesley)

Pull request description:

Decrementing psz beyond the beginning of the string is UB, even though
the out-of-bounds pointer is never dereferenced.

I don't think any clang sanitizer covers this, so I don't see any way a test could catch the original behavior.

ACKs for top commit:

promag:
  utACK 0f459d8.
l2a5b1:
  utACK 0f459d868d85053f1cc066ea9099793f88cbd655

Tree-SHA512: 388223254ea6e955f643d2ebdf74d15a3d494e9f0597d9f05987ebb708d7a1cc06ce64bd25d447d75b5f5561bdae9630dcf25adb7bd75f7a382298b95d127162

Backport of Core PR14734

Test Plan

ninja check

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

This revision is now accepted and ready to land.Jul 7 2020, 19:18
majcosta requested changes to this revision.Jul 7 2020, 19:20
majcosta added inline comments.
src/uint256.cpp
45 ↗(On Diff #22056)

Does this change behavior? Should be uint8_t(::HexDigit(psz[--digits])) << 4

This revision now requires changes to proceed.Jul 7 2020, 19:20
src/uint256.cpp
45 ↗(On Diff #22056)

I'll fix it to be completely in line with Core's code, but no it doesn't change behavior.
The largest return value of HexDigit() is 15 (15 << 4 = 15 * 4 = 60). Fits just fine in a char.

Fix casting to better align with Core

This revision is now accepted and ready to land.Jul 7 2020, 22:01