Merge #18413: script: prevent UB when computing abs value for num opcode serialize
2748e87932
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: ACK2748e87932
MarcoFalke: ACK2748e87932
, only checked that the bitcoind binary does not change with clang -O2 🎓 practicalswift: ACK2748e87932
Tree-SHA512: 539a34c636c2674c66cb6e707d9d0dfdce63f59b5525610ed88da10c9a8d59d81466b111ad63b850660cef3750d732fc7755530c81a2d61f396be0707cd86dec
This commit is contained in:
commit
68ef9523d1
3 changed files with 3 additions and 11 deletions
|
@ -329,7 +329,7 @@ public:
|
||||||
|
|
||||||
std::vector<unsigned char> result;
|
std::vector<unsigned char> result;
|
||||||
const bool neg = value < 0;
|
const bool neg = value < 0;
|
||||||
uint64_t absvalue = neg ? -value : value;
|
uint64_t absvalue = neg ? ~static_cast<uint64_t>(value) + 1 : static_cast<uint64_t>(value);
|
||||||
|
|
||||||
while(absvalue)
|
while(absvalue)
|
||||||
{
|
{
|
||||||
|
|
|
@ -148,11 +148,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
|
||||||
|
|
||||||
const CScriptNum script_num{i64};
|
const CScriptNum script_num{i64};
|
||||||
(void)script_num.getint();
|
(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);
|
const arith_uint256 au256 = UintToArith256(u256);
|
||||||
assert(ArithToUint256(au256) == u256);
|
assert(ArithToUint256(au256) == u256);
|
||||||
|
|
|
@ -129,10 +129,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
(void)script_num.getint();
|
(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();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
Loading…
Reference in a new issue