Multiple SIGILL crashes in BT::ValidCast due to unsafe type conversion comparisons

[cktii]

Description

Multiple undefined behavior conditions found in BT::ValidCast template function when performing numeric type conversions. The function attempts to compare values across different numeric types without checking value ranges, leading to illegal instructions (SIGILL) when handling extreme values.

BehaviorTree.CPP/include/behaviortree_cpp/utils/safe_any.hpp

Lines 245 to 302 in 48f6c5b

	template <typename SRC, typename TO>
	inline bool ValidCast(const SRC& val)
	{
	return (val == static_cast<SRC>(static_cast<TO>(val)));
	}
	template <typename T>
	inline bool isCastingSafe(const std::type_index& type, const T& val)
	{
	if(type == typeid(T))
	{
	return true;
	}
	if(std::type_index(typeid(uint8_t)) == type)
	{
	return ValidCast<T, uint8_t>(val);
	}
	if(std::type_index(typeid(uint16_t)) == type)
	{
	return ValidCast<T, uint16_t>(val);
	}
	if(std::type_index(typeid(uint32_t)) == type)
	{
	return ValidCast<T, uint32_t>(val);
	}
	if(std::type_index(typeid(uint64_t)) == type)
	{
	return ValidCast<T, uint64_t>(val);
	}
	//------------
	if(std::type_index(typeid(int8_t)) == type)
	{
	return ValidCast<T, int8_t>(val);
	}
	if(std::type_index(typeid(int16_t)) == type)
	{
	return ValidCast<T, int16_t>(val);
	}
	if(std::type_index(typeid(int32_t)) == type)
	{
	return ValidCast<T, int32_t>(val);
	}
	if(std::type_index(typeid(int64_t)) == type)
	{
	return ValidCast<T, int64_t>(val);
	}
	//------------
	if(std::type_index(typeid(float)) == type)
	{
	return ValidCast<T, float>(val);
	}
	if(std::type_index(typeid(double)) == type)
	{
	return ValidCast<T, double>(val);
	}
	return false;
	}

Found in commit: 48f6c5b

Bug Class

Type Conversion Vulnerability - Undefined Behavior

Impact

• Program crashes (SIGILL) across multiple numeric type conversions
• Affects type safety checks throughout the blackboard system
• Can be triggered during normal type conversion operations
• Potential security implications when processing untrusted data

Types Affected

Found crashes in the following ValidCast instantiations:

• ValidCast<double, int>
• ValidCast<double, unsigned long>
• ValidCast<float, int>
• ValidCast<float, unsigned long>
• ValidCast<int, float>
• ValidCast<unsigned long, double>
• ValidCast<unsigned long, float>

Root Cause

In safe_any.hpp:248:

template <typename SRC, typename TO>
inline bool ValidCast(const SRC& val)
{
  return (val == static_cast<SRC>(static_cast<TO>(val)));  // UB here
}

The issue occurs because:

• The inner static_cast<TO> can be undefined behavior when val is outside TO's range
• The subsequent static_cast<SRC> back can also be undefined behavior
• The comparison is performed with potentially invalid values from undefined behavior

Example Stack trace

#0  0x55555571243a in BT::ValidCast<unsigned long, double> (val=18446744073709551370)
#1  0x55555570f72e in BT::isCastingSafe<unsigned long>
#2  0x5555556bca1d in BT::Blackboard::set<unsigned long>
#3  0x5555556ac24d in BlackboardFuzzer::fuzzSingleBB

Proposed Fix

template <typename SRC, typename TO>
inline bool ValidCast(const SRC& val)
{
    // First check numeric limits
    if constexpr(std::is_arithmetic_v<SRC> && std::is_arithmetic_v<TO>) {
        // Handle conversion to floating point
        if constexpr(std::is_floating_point_v<TO>) {
            if constexpr(std::is_integral_v<SRC>) {
                // For integral to float, check if we can represent the value exactly
                TO as_float = static_cast<TO>(val);
                SRC back_conv = static_cast<SRC>(as_float);
                return back_conv == val;
            }
        }
        // Handle conversion to integral
        else if constexpr(std::is_integral_v<TO>) {
            if (val > static_cast<SRC>(std::numeric_limits<TO>::max()) ||
                val < static_cast<SRC>(std::numeric_limits<TO>::min())) {
                return false;
            }
        }
    }
    
    TO as_target = static_cast<TO>(val);
    SRC back_to_source = static_cast<SRC>(as_target);
    return val == back_to_source;
}

Additional Notes

This was found via fuzzing with AFL++. The crash is reproducible in both Debug and Release builds.

[facontidavide]

Thanks a lot for the help.

Do you mind creating a PR? Also, I think you should use static_cast<SRC>(std::numeric_limits<TO>::lowest() instead of ::min()

[facontidavide]

solved. thanks!