Regression: integer overflow in ReadVarBytes reintroduced by v1.83 TCG merge (commit ee21db0) - fix from PR #100 was reverted.

[geeky-ashu]

The integer overflow fix in ReadVarBytes that was merged in PR #100 (commit c3c41c0, Sep 2023) was silently reverted by the v1.83 TCG integration in commit ee21db0 (Oct 2024). The current code in main is back to the vulnerable state.

Current code in TPMCmd/Simulator/src/TcpServer.c:

bool ReadVarBytes(SOCKET s, char* buffer, uint32_t* BytesReceived, int MaxLen)
{
    int  length;
    ...
    length = ntohl(length);
    *BytesReceived = length;
    if(length > MaxLen)   // passes if length is negative
        return false;
    ...
    res = ReadBytes(s, buffer, length);  // loop condition 0 < negative is false, returns immediately

The problem is identical to what PR #100 described. If a client sends 0xFFFFFFFF as the 4-byte length field, ntohl assigns it to a signed int, giving length = -1. The bounds check passes because -1 < MaxLen. ReadBytes returns true immediately because the while(numGot < NumBytes) loop condition is false for negative NumBytes. BytesReceived is then set to (uint32_t)(-1) = 4294967295, and the caller proceeds to use that as a valid buffer length against an empty buffer.

The fix from PR #100 was to change int length to uint32_t length and int MaxLen to uint32_t MaxLen. That fix needs to be reapplied.