diff options
author | fanquake <fanquake@gmail.com> | 2020-05-02 20:52:15 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2020-05-02 21:24:05 +0800 |
commit | 68ef9523d1bcd00afbccee2a6585c9f82ddcdb31 (patch) | |
tree | 629945c4ce4f60d94e0926c969895619e8539856 /src/test | |
parent | 844d2070a2c0106bb7a54be5cad7d4da4d9cd55e (diff) | |
parent | 2748e8793267126c5b40621d75d1930e358f057e (diff) |
Merge #18413: script: prevent UB when computing abs value for num opcode serialize
2748e8793267126c5b40621d75d1930e358f057e script: prevent UB when computing abs value for num opcode serialize (pierrenn)
Pull request description:
This was reported by practicalswift here #18046
It seems that the original author of the line used a reference to glibc `abs`: https://github.com/lattera/glibc/blob/master/stdlib/abs.c
However depending on some implementation details this can be undefined behavior for unusual values.
A detailed explanation of the UB is provided here : https://stackoverflow.com/questions/17313579/is-there-a-safe-way-to-get-the-unsigned-absolute-value-of-a-signed-integer-with (by [Billy O'Neal](https://twitter.com/malwareminigun))
Simple relevant godbolt example : https://godbolt.org/z/yRwtCG
Thanks!
ACKs for top commit:
sipa:
ACK 2748e8793267126c5b40621d75d1930e358f057e
MarcoFalke:
ACK 2748e8793267126c5b40621d75d1930e358f057e, only checked that the bitcoind binary does not change with clang -O2 🎓
practicalswift:
ACK 2748e8793267126c5b40621d75d1930e358f057e
Tree-SHA512: 539a34c636c2674c66cb6e707d9d0dfdce63f59b5525610ed88da10c9a8d59d81466b111ad63b850660cef3750d732fc7755530c81a2d61f396be0707cd86dec
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/fuzz/integer.cpp | 6 | ||||
-rw-r--r-- | src/test/fuzz/scriptnum_ops.cpp | 6 |
2 files changed, 2 insertions, 10 deletions
diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index d5b5ec3c9a..35d6804d4f 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -148,11 +148,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) const CScriptNum script_num{i64}; (void)script_num.getint(); - // Avoid negation failure: - // script/script.h:332:35: runtime error: negation of -9223372036854775808 cannot be represented in type 'int64_t' (aka 'long'); cast to an unsigned type to negate this value to itself - if (script_num != CScriptNum{std::numeric_limits<int64_t>::min()}) { - (void)script_num.getvch(); - } + (void)script_num.getvch(); const arith_uint256 au256 = UintToArith256(u256); assert(ArithToUint256(au256) == u256); diff --git a/src/test/fuzz/scriptnum_ops.cpp b/src/test/fuzz/scriptnum_ops.cpp index 42b1432f13..f4e079fb89 100644 --- a/src/test/fuzz/scriptnum_ops.cpp +++ b/src/test/fuzz/scriptnum_ops.cpp @@ -129,10 +129,6 @@ void test_one_input(const std::vector<uint8_t>& buffer) break; } (void)script_num.getint(); - // Avoid negation failure: - // script/script.h:332:35: runtime error: negation of -9223372036854775808 cannot be represented in type 'int64_t' (aka 'long'); cast to an unsigned type to negate this value to itself - if (script_num != CScriptNum{std::numeric_limits<int64_t>::min()}) { - (void)script_num.getvch(); - } + (void)script_num.getvch(); } } |