Possible integer overflow when converting an ImgDetection to a ROS message

[cktii]

Introduction

Hi,

I'm working on setting up fuzzing for the public APIs that this application has to offer to improve the robustness and security of the overall system.
While doing so, I ran into the below described issue.

• Tested commit: 698d227

Bug Summary

Under certain conditions, there's a chance that within the ImgDetectionConverter there is a possible integer overflow due to a corrupted timestamp in a dai::ImgDetection structure. The issue can manifest when calling the toRosMsg() API.

In particularly, within the ImgDetectionConverter there is the following API:

void ImgDetectionConverter::toRosMsg(
  std::shared_ptr<dai::ImgDetections> inNetData, // assume compromised/corrupted
  std::deque<VisionMsgs::Detection2DArray> & opDetectionMsgs)
{
  // ...
  auto tstamp = inNetData->getTimestamp(); // tstamp tainted based on assumed corrupted input
  VisionMsgs::Detection2DArray opDetectionMsg;

  opDetectionMsg.header.stamp = getFrameTime(_rosBaseTime, _steadyBaseTime, tstamp); // issue here, as tstamp is supplied as an argument
  // ...
}

This API takes in n ImgDetections and eventually calls into a utility function (defined in depthaiUtility.hpp), called getFrameTime in which the integer overflow can occur:

inline rclcpp::Time getFrameTime(
  rclcpp::Time rclBaseTime,
  std::chrono::time_point<std::chrono::steady_clock> steadyBaseTime,
  std::chrono::time_point<std::chrono::steady_clock,
  std::chrono::steady_clock::duration> currTimePoint)  // currTimePoint tainted
{
  auto elapsedTime = currTimePoint - steadyBaseTime; // tainted
  auto rclStamp = rclBaseTime + elapsedTime; // crash here
  return rclStamp;
}

Stack trace

/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:716:34: runtime error: signed integer overflow: -9223372036854775808 - 453060024732859 cannot be represented in type 'rep' (aka 'long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:716:34
terminate called after throwing an instance of 'std::overflow_error'
  what():  addition leads to int64_t overflow
Stack trace (most recent call last):
#18   Object "", at 0xffffffffffffffff, in
#17   Object "/home/user/git/work/depthai_ctrl/build/depthai_ctrl/img_det_conv_fuzz", at 0x5e57be11ac54, in _start
#16   Source "../csu/libc-start.c", line 360, in __libc_start_main_impl [0x73e80102a28a]
#15   Source "../sysdeps/nptl/libc_start_call_main.h", line 58, in __libc_start_call_main [0x73e80102a1c9]
#14   Object "/home/user/git/work/depthai_ctrl/build/depthai_ctrl/img_det_conv_fuzz", at 0x5e57be1502f6, in main
#13   Object "/home/user/git/work/depthai_ctrl/build/depthai_ctrl/img_det_conv_fuzz", at 0x5e57be125c6f, in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))
#12   Object "/home/user/git/work/depthai_ctrl/build/depthai_ctrl/img_det_conv_fuzz", at 0x5e57be138777, in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile> >&)
#11   Object "/home/user/git/work/depthai_ctrl/build/depthai_ctrl/img_det_conv_fuzz", at 0x5e57be138181, in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile> >&)
#10   Object "/home/user/git/work/depthai_ctrl/build/depthai_ctrl/img_det_conv_fuzz", at 0x5e57be136f54, in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)
#9    Source "/home/user/git/work/depthai_ctrl/fuzz/img_det_conv_fuzz.cc", line 58, in LLVMFuzzerTestOneInput [0x5e57be22c640]
         56:   std::deque<dai::ros::VisionMsgs::Detection2DArray> opDetectionMsgs;
         57:
      >  58:   detectionConverter.toRosMsg(inNetData, opDetectionMsgs);
         59:   auto rosMsgPtr = detectionConverter.toRosMsgPtr(inNetData);
         60:   // try {
         61:   // } catch (const std::exception &e) {
#8  | Source "/home/user/git/work/depthai_ctrl/src/ImgDetectionConverter.cpp", line 28, in getFrameTime
    |    26:   VisionMsgs::Detection2DArray opDetectionMsg;
    |    27:
    | >  28:   opDetectionMsg.header.stamp = getFrameTime(_rosBaseTime, _steadyBaseTime, tstamp);
    |    29:   opDetectionMsg.header.frame_id = _frameName;
    |    30:   opDetectionMsg.detections.resize(inNetData->detections.size());
      Source "/home/user/git/work/depthai_ctrl/include/depthai_ctrl/depthaiUtility.hpp", line 117, in toRosMsg [0x73e801316bac]
        114: {
        115:   auto elapsedTime = currTimePoint - steadyBaseTime;
        116:   // uint64_t nSec = rosBaseTime.toNSec() + std::chrono::duration_cast<std::chrono::nanoseconds>(elapsedTime).count();
      > 117:   auto rclStamp = rclBaseTime + elapsedTime;
        118:   // DEPTHAI_ROS_DEBUG_STREAM("PRINT TIMESTAMP: ", "rosStamp -> " << rclStamp << "  rosBaseTime -> " << rclBaseTime);
        119:   return rclStamp;
        120: }
#7    Object "/opt/ros/jazzy/lib/librclcpp.so", at 0x73e8006e31f9, in
#6    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.33", at 0x73e8014bb127, in __cxa_throw
#5    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.33", at 0x73e8014a5a48, in std::terminate()
#4    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.33", at 0x73e8014bae9b, in
#3    Object "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.33", at 0x73e8014a5ffd, in
#2    Source "./stdlib/abort.c", line 79, in abort [0x73e8010288fe]
#1    Source "../sysdeps/posix/raise.c", line 26, in raise [0x73e80104526d]
#0  | Source "./nptl/pthread_kill.c", line 89, in __pthread_kill_internal
    | Source "./nptl/pthread_kill.c", line 78, in __pthread_kill_implementation
      Source "./nptl/pthread_kill.c", line 44, in __pthread_kill [0x73e80109eb1c]
Aborted (Signal sent by tkill() 578185 1000)
[1]    578185 IOT instruction  ./build/depthai_ctrl/img_det_conv_fuzz -use_value_profile=1

Reproduction Steps

If desired, I can share the fuzzing setup or contribute the fuzzer to the repository itself so it can be run by anyone, or even in the CI/CD.

Expected Behavior

With the assumption that a dai::ImgDetection could be corrupted/tampered with, the library should handle this case gracefully

Actual Behavior

As shown in the stack-trace, when enabling UBSAN we can quickly catch that issue and the process crashes.

Impact

Similar to #84 I'm currently still unfamiliar with how and if this functionality is used in our ecosystem and whether attacker/user-supplied data ever reaches this API, but as it is also compiled as part of the depthai_bridge library based on this project CMakeLists.txt. Hence, I'm under the assumption that it is actively used and could lead to issues in certain scenarios.

Possible Fix

• Sanitize the timestamp or ensure that it fits into the current scope, e.g. if the timestamp is arbitrary but the ImgDetection data should be from like a mission today, a fallback default value could be used
• Ensure that both in the addition and subtraction, no integer overflow/underflow can occur due to possible corrupted input.

Additional Information

Duplicated in JIRA: PTAD-51