fix(rx): reject undersized typed frames safely

PacketTypes.cpp

@@ -2,12 +2,30 @@
 
 namespace PacketTypes
 {
+    bool BasePack::checkPacketLayout() const
+    {
+        if (packInfo == nullptr || packInfo->buffer == nullptr ||
+            packInfo->packSize < msgBytes + crcBytes ||
+            packInfo->packSize > irproto::kMaxWireFrameBytes)
+        {
+            return false;
+        }
+        return (packInfo->buffer[msgOffset] & IR_MASK_MSG_INFO) == packInfo->packSize;
+    }
+
     bool BasePack::checkAddress() { return true; };
     void BasePack::set(IR_FOX::PackInfo *packInfo, uint16_t id)
     {
+        isAvailable = false;
+        isRawAvailable = false;
         this->packInfo = packInfo;
         this->id = id;
 
+        if (!checkPacketLayout())
+        {
+            return;
+        }
+
         if (checkAddress())
         {
             isAvailable = true;
@@ -27,24 +45,58 @@ namespace PacketTypes
 
     uint16_t BasePack::_getAddrFrom(BasePack *obj)
     {
+        if (obj == nullptr || !obj->checkPacketLayout() ||
+            obj->packInfo == nullptr || obj->packInfo->buffer == nullptr ||
+            obj->packInfo->packSize < crcBytes ||
+            static_cast<uint16_t>(obj->addressFromOffset) + 1U >=
+                static_cast<uint16_t>(obj->packInfo->packSize - crcBytes))
+        {
+            return 0U;
+        }
         return (obj->packInfo->buffer[obj->addressFromOffset] << 8) | obj->packInfo->buffer[obj->addressFromOffset + 1];
     };
     uint16_t BasePack::_getAddrTo(BasePack *obj)
     {
+        if (obj == nullptr || !obj->checkPacketLayout() ||
+            obj->packInfo == nullptr || obj->packInfo->buffer == nullptr ||
+            obj->packInfo->packSize < crcBytes ||
+            static_cast<uint16_t>(obj->addressToOffset) + 1U >=
+                static_cast<uint16_t>(obj->packInfo->packSize - crcBytes))
+        {
+            return 0U;
+        }
         return (obj->packInfo->buffer[obj->addressToOffset] << 8) | obj->packInfo->buffer[obj->addressToOffset + 1];
     };
 
     uint8_t BasePack::_getDataSize(BasePack *obj)
     {
-        return obj->packInfo->packSize - crcBytes - obj->DataOffset;
+        if (obj == nullptr || !obj->checkPacketLayout() ||
+            obj->packInfo == nullptr || obj->packInfo->buffer == nullptr)
+        {
+            return 0U;
+        }
+        const uint16_t overhead = static_cast<uint16_t>(obj->DataOffset) + crcBytes;
+        return static_cast<uint16_t>(obj->packInfo->packSize) > overhead
+                   ? static_cast<uint8_t>(static_cast<uint16_t>(obj->packInfo->packSize) - overhead)
+                   : 0U;
     };
     uint8_t *BasePack::_getDataPrt(BasePack *obj)
     {
+        if (obj == nullptr || !obj->checkPacketLayout() ||
+            obj->packInfo == nullptr || obj->packInfo->buffer == nullptr ||
+            obj->packInfo->packSize < crcBytes ||
+            static_cast<uint16_t>(obj->DataOffset) >
+                static_cast<uint16_t>(obj->packInfo->packSize - crcBytes))
+        {
+            return nullptr;
+        }
         return obj->packInfo->buffer + obj->DataOffset;
     };
     uint8_t BasePack::_getDataRawSize(BasePack *obj)
     {
-        return obj->packInfo->packSize;
+        return obj != nullptr && obj->checkPacketLayout() && obj->packInfo != nullptr
+                   ? obj->packInfo->packSize
+                   : 0U;
     };
 
     bool BasePack::available()
@@ -73,6 +125,17 @@ namespace PacketTypes
         }
     };
 
+    bool Data::checkPacketLayout() const
+    {
+        if (!BasePack::checkPacketLayout())
+        {
+            return false;
+        }
+        const uint8_t msgType = (packInfo->buffer[msgOffset] >> 5) & IR_MASK_MSG_TYPE;
+        return (msgType == IR_MSG_DATA_ACCEPT || msgType == IR_MSG_DATA_NOACCEPT) &&
+               irproto::isTypedWireSizeValid(msgType, packInfo->packSize);
+    }
+
     bool Data::checkAddress()
     {
         bool ret;
@@ -80,6 +143,17 @@ namespace PacketTypes
         return ret;
     }
 
+    bool DataBack::checkPacketLayout() const
+    {
+        if (!BasePack::checkPacketLayout())
+        {
+            return false;
+        }
+        const uint8_t msgType = (packInfo->buffer[msgOffset] >> 5) & IR_MASK_MSG_TYPE;
+        return (msgType == IR_MSG_BACK || msgType == IR_MSG_BACK_TO) &&
+               irproto::isTypedWireSizeValid(msgType, packInfo->packSize);
+    }
+
     bool DataBack::checkAddress()
     {
         bool ret;
@@ -96,8 +170,30 @@ namespace PacketTypes
         return ret;
     }
 
+    bool Accept::checkPacketLayout() const
+    {
+        if (!BasePack::checkPacketLayout())
+        {
+            return false;
+        }
+        const uint8_t msgType = (packInfo->buffer[msgOffset] >> 5) & IR_MASK_MSG_TYPE;
+        return msgType == IR_MSG_ACCEPT &&
+               irproto::isTypedWireSizeValid(msgType, packInfo->packSize);
+    }
+
     bool Accept::checkAddress() { return true; }
 
+    bool Request::checkPacketLayout() const
+    {
+        if (!BasePack::checkPacketLayout())
+        {
+            return false;
+        }
+        const uint8_t msgType = (packInfo->buffer[msgOffset] >> 5) & IR_MASK_MSG_TYPE;
+        return msgType == IR_MSG_REQUEST &&
+               irproto::isTypedWireSizeValid(msgType, packInfo->packSize);
+    }
+
     bool Request::checkAddress()
     {
         bool ret;

PacketTypes.h

@@ -9,18 +9,19 @@ namespace PacketTypes
         friend IR_Decoder;
 
     protected:
-        bool isAvailable;
+        bool isAvailable = false;
-        bool isRawAvailable;
+        bool isRawAvailable = false;
-        bool isNeedAccept;
+        bool isNeedAccept = false;
 
-        uint8_t msgOffset;
+        uint8_t msgOffset = 0;
-        uint8_t addressFromOffset;
+        uint8_t addressFromOffset = 0;
-        uint8_t addressToOffset;
+        uint8_t addressToOffset = 0;
-        uint8_t DataOffset;
+        uint8_t DataOffset = 0;
 
-        IR_FOX::PackInfo *packInfo;
+        IR_FOX::PackInfo *packInfo = nullptr;
-        uint16_t id;
+        uint16_t id = 0;
 
+        virtual bool checkPacketLayout() const;
         virtual bool checkAddress();
         void set(IR_FOX::PackInfo *packInfo, uint16_t id);
 
@@ -34,9 +35,9 @@ namespace PacketTypes
         bool available();
         bool availableRaw();
 
-        inline uint8_t getMsgInfo() { return packInfo->buffer[0] & IR_MASK_MSG_INFO; };
+        inline uint8_t getMsgInfo() { return packInfo != nullptr && packInfo->buffer != nullptr ? packInfo->buffer[0] & IR_MASK_MSG_INFO : 0U; };
-        inline uint8_t getMsgType() { return (packInfo->buffer[0] >> 5) & IR_MASK_MSG_TYPE; };
+        inline uint8_t getMsgType() { return packInfo != nullptr && packInfo->buffer != nullptr ? (packInfo->buffer[0] >> 5) & IR_MASK_MSG_TYPE : 0U; };
-        inline uint8_t getMsgRAW() { return packInfo->buffer[0]; };
+        inline uint8_t getMsgRAW() { return packInfo != nullptr && packInfo->buffer != nullptr ? packInfo->buffer[0] : 0U; };
         inline uint16_t getErrorCount() { return packInfo->err.all(); };
         inline uint8_t getErrorLowSignal() { return packInfo->err.lowSignal; };
         inline uint8_t getErrorHighSignal() { return packInfo->err.highSignal; };
@@ -65,6 +66,7 @@ namespace PacketTypes
         inline uint8_t *getDataPrt() { return _getDataPrt(this); };
 
     private:
+        bool checkPacketLayout() const override;
         bool checkAddress() override;
     };
 
@@ -86,6 +88,7 @@ namespace PacketTypes
         inline uint8_t *getDataPrt() { return _getDataPrt(this); };
 
     private:
+        bool checkPacketLayout() const override;
         bool checkAddress() override;
     };
 
@@ -103,6 +106,7 @@ namespace PacketTypes
        inline uint8_t getCustomByte() { return packInfo->buffer[DataOffset]; };
 
     private:
+        bool checkPacketLayout() const override;
         bool checkAddress() override;
     };
 
@@ -121,6 +125,7 @@ namespace PacketTypes
         inline uint16_t getAddrTo() { return _getAddrTo(this); };
 
     private:
+        bool checkPacketLayout() const override;
         bool checkAddress() override;
     };
 

tests/run_host_tests.ps1

@@ -0,0 +1,38 @@
+$ErrorActionPreference = 'Stop'
+
+$repo = Split-Path -Parent $PSScriptRoot
+$build = Join-Path $PSScriptRoot '.build'
+New-Item -ItemType Directory -Force -Path $build | Out-Null
+
+$compiler = if (Test-Path -LiteralPath 'C:\MinGW\bin\g++.exe') {
+    'C:\MinGW\bin\g++.exe'
+} else {
+    (Get-Command g++ -ErrorAction Stop).Source
+}
+
+$common = @(
+    '-std=c++17', '-Wall', '-Wextra', '-Werror',
+    '-Wno-unused-parameter', '-Wno-ignored-qualifiers', '-Wno-sign-compare',
+    '-I', (Join-Path $PSScriptRoot 'arduino_stubs'),
+    '-I', $repo
+)
+
+& $compiler @common `
+    (Join-Path $PSScriptRoot 'test_timing_contract.cpp') `
+    (Join-Path $repo 'IR_Encoder.cpp') `
+    (Join-Path $repo 'IR_config.cpp') `
+    '-o' (Join-Path $build 'test_timing_contract.exe')
+if ($LASTEXITCODE -ne 0) { throw 'timing test build failed' }
+
+& (Join-Path $build 'test_timing_contract.exe')
+if ($LASTEXITCODE -ne 0) { throw 'timing test failed' }
+
+& $compiler @common `
+    (Join-Path $PSScriptRoot 'test_packet_types.cpp') `
+    (Join-Path $repo 'PacketTypes.cpp') `
+    (Join-Path $repo 'IR_config.cpp') `
+    '-o' (Join-Path $build 'test_packet_types.exe')
+if ($LASTEXITCODE -ne 0) { throw 'packet test build failed' }
+
+& (Join-Path $build 'test_packet_types.exe')
+if ($LASTEXITCODE -ne 0) { throw 'packet test failed' }

tests/test_packet_types.cpp

@@ -0,0 +1,145 @@
+#include "PacketTypes.h"
+
+#include <array>
+#include <cassert>
+#include <cstdint>
+#include <iostream>
+
+namespace
+{
+template <typename Packet>
+class ExposedPacket : public Packet
+{
+public:
+    void attach(IR_FOX::PackInfo *info, uint16_t id = 0U)
+    {
+        this->set(info, id);
+    }
+};
+
+IR_FOX::PackInfo makeFrame(uint8_t *buffer, uint8_t msgType, uint8_t wireBytes)
+{
+    buffer[0] = static_cast<uint8_t>((msgType << 5) | (wireBytes & IR_MASK_MSG_INFO));
+    IR_FOX::PackInfo result;
+    result.buffer = buffer;
+    result.packSize = wireBytes;
+    return result;
+}
+
+template <typename Packet>
+void verifyMinimum(uint8_t msgType, uint8_t minimum)
+{
+    std::array<uint8_t, irproto::kMaxWireFrameBytes> buffer{};
+    ExposedPacket<Packet> packet;
+
+    IR_FOX::PackInfo shortFrame = makeFrame(buffer.data(), msgType, minimum - 1U);
+    packet.attach(&shortFrame);
+    assert(!packet.available());
+    assert(!packet.availableRaw());
+
+    IR_FOX::PackInfo minimumFrame = makeFrame(buffer.data(), msgType, minimum);
+    packet.attach(&minimumFrame);
+    assert(packet.available());
+}
+
+void verifyTypedMinimums()
+{
+    verifyMinimum<PacketTypes::Data>(IR_MSG_DATA_ACCEPT, 7U);
+    verifyMinimum<PacketTypes::Data>(IR_MSG_DATA_NOACCEPT, 7U);
+    verifyMinimum<PacketTypes::DataBack>(IR_MSG_BACK, 5U);
+    verifyMinimum<PacketTypes::DataBack>(IR_MSG_BACK_TO, 7U);
+    verifyMinimum<PacketTypes::Accept>(IR_MSG_ACCEPT, 6U);
+    verifyMinimum<PacketTypes::Request>(IR_MSG_REQUEST, 7U);
+}
+
+void verifyDataAccessCannotUnderflow()
+{
+    std::array<uint8_t, irproto::kMaxWireFrameBytes> buffer{};
+    ExposedPacket<PacketTypes::Data> data;
+
+    for (uint8_t wireBytes = 0U; wireBytes < irproto::kDataFrameOverheadBytes; ++wireBytes)
+    {
+        IR_FOX::PackInfo malformed = makeFrame(buffer.data(), IR_MSG_DATA_ACCEPT, wireBytes);
+        data.attach(&malformed);
+        assert(!data.available());
+        assert(data.getDataSize() == 0U);
+        assert(data.getDataPrt() == nullptr);
+        assert(data.getAddrTo() == 0U);
+    }
+
+    IR_FOX::PackInfo empty = makeFrame(buffer.data(), IR_MSG_DATA_ACCEPT, 7U);
+    data.attach(&empty);
+    assert(data.available());
+    assert(data.getDataSize() == 0U);
+    assert(data.getDataPrt() == buffer.data() + 5U);
+
+    IR_FOX::PackInfo oneByte = makeFrame(buffer.data(), IR_MSG_DATA_ACCEPT, 8U);
+    data.attach(&oneByte);
+    assert(data.available());
+    assert(data.getDataSize() == 1U);
+    assert(data.getDataPrt() == buffer.data() + 5U);
+}
+
+void verifyBackLayouts()
+{
+    std::array<uint8_t, irproto::kMaxWireFrameBytes> buffer{};
+    ExposedPacket<PacketTypes::DataBack> back;
+
+    IR_FOX::PackInfo shortBroadcast = makeFrame(buffer.data(), IR_MSG_BACK, 4U);
+    back.attach(&shortBroadcast);
+    assert(!back.available());
+    assert(back.getDataSize() == 0U);
+    assert(back.getDataPrt() == nullptr);
+
+    IR_FOX::PackInfo broadcast = makeFrame(buffer.data(), IR_MSG_BACK, 5U);
+    back.attach(&broadcast);
+    assert(back.available());
+    assert(back.getDataSize() == 0U);
+    assert(back.getDataPrt() == buffer.data() + 3U);
+
+    IR_FOX::PackInfo shortAddressed = makeFrame(buffer.data(), IR_MSG_BACK_TO, 6U);
+    back.attach(&shortAddressed);
+    assert(!back.available());
+    assert(back.getDataSize() == 0U);
+
+    IR_FOX::PackInfo addressed = makeFrame(buffer.data(), IR_MSG_BACK_TO, 7U);
+    back.attach(&addressed);
+    assert(back.available());
+    assert(back.getDataSize() == 0U);
+    assert(back.getDataPrt() == buffer.data() + 5U);
+}
+
+void verifyRawAndHeaderContracts()
+{
+    std::array<uint8_t, irproto::kMaxWireFrameBytes> buffer{};
+    ExposedPacket<PacketTypes::BasePack> raw;
+
+    // Raw diagnostics remain able to observe a CRC-sized frame even when its
+    // declared type-specific layout is too short.
+    IR_FOX::PackInfo shortTyped = makeFrame(buffer.data(), IR_MSG_DATA_ACCEPT, 3U);
+    raw.attach(&shortTyped);
+    assert(raw.availableRaw());
+
+    IR_FOX::PackInfo inconsistent = makeFrame(buffer.data(), IR_MSG_BACK, 5U);
+    inconsistent.packSize = 6U;
+    raw.attach(&inconsistent);
+    assert(!raw.available());
+    assert(!raw.availableRaw());
+
+    IR_FOX::PackInfo nullFrame;
+    nullFrame.packSize = 31U;
+    raw.attach(&nullFrame);
+    assert(!raw.available());
+    assert(!raw.availableRaw());
+}
+}
+
+int main()
+{
+    verifyTypedMinimums();
+    verifyDataAccessCannotUnderflow();
+    verifyBackLayouts();
+    verifyRawAndHeaderContracts();
+    std::cout << "IR packet boundary tests: OK\n";
+    return 0;
+}