Skip to content

fix undefined double to int64 cast in the script complement operator - #1239

Open
aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:script-complement-cast-ub
Open

aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:script-complement-cast-ub

Conversation

@aysha-afrah26

Copy link
Copy Markdown
Contributor

The scripting operator ~ is evaluated in ExprUnaryArithmetic::evaluate (operators.hpp) by casting the operand to double and then to int64_t behind a hand-written range check, rv > double(INT64_MAX) || rv < double(INT64_MIN). That check lets NaN through, since every comparison with NaN is false, and it lets exactly 2^63 through, since double(INT64_MAX) rounds up to 2^63 and 2^63 is not greater than itself; both reach static_cast<int64_t>, which is undefined for values outside the range, and it also truncates fractional operands silently, so ~1.5 evaluates as ~1. The binary bitwise operators a few lines below go through the checked Any::cast<int64_t> instead and reject a non-integer, so the complement is the one remaining hand-rolled cast in the evaluator (the shared helpers it now relies on were tightened at the boundary in #1162). The operand is untrusted script text from Script nodes and _skipIf/_while/_onSuccess attributes, or a blackboard double: ~(0.0 / 0.0) and ~9223372036854775808.0 both trigger UBSan float-cast-overflow at operators.hpp:137 in the ASan+UBSan configuration the CI runs. The fix routes the complement through the same checked cast the binary operators use, so NaN, infinities, out-of-range and fractional values raise the existing RuntimeError and integral values, including both int64_t boundaries, evaluate as before. Header-only, no API or ABI change. The new test ParserTest.BitwiseComplementRejectsNonIntegers aborts under UBSan on master and passes with the change.

The ~ operator in ExprUnaryArithmetic::evaluate guarded the cast of its
double operand to int64_t with a hand-written range comparison. NaN passes
it (every comparison is false) and so does exactly 2^63, because
double(INT64_MAX) rounds up to 2^63; the following static_cast is then
undefined behavior (UBSan float-cast-overflow). Fractional operands were
also truncated silently, while the binary bitwise operators reject them.

Use the checked Any::cast<int64_t> the binary bitwise operators already
use, so these operands raise the existing RuntimeError instead.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant