GameNetworkingSockets icon indicating copy to clipboard operation
GameNetworkingSockets copied to clipboard

fix(feat): Multiplication result converted to larger type

Open odaysec opened this issue 7 months ago • 2 comments

https://github.com/ValveSoftware/GameNetworkingSockets/blob/725e273c7442bac7a8bc903c0b210b1c15c34d92/src/tier1/utlmemory.cpp#L21-L21

fix the issue need to ensure that the multiplication is performed using a larger type (e.g., size_t) to prevent overflow. This can be achieved by explicitly casting one of the operands to size_t before the multiplication. This ensures that the multiplication is performed in the larger type, avoiding overflow.

The specific change involves modifying line 21 to cast m_nAllocationCount or m_unSizeOfElements to size_t before the multiplication. This change is localized and does not affect the rest of the code's functionality.

The product performs a calculation that can produce an integer overflow or wraparound when the logic assumes that the resulting value will always be larger than the original value. This occurs when an integer value is incremented to a value that is too large to store in the associated representation. When this occurs, the value may become a very small or negative number.

CWE-190-Diagram

This rule finds code that converts the result of an integer multiplication to a larger type. Since the conversion applies after the multiplication, arithmetic overflow may still occur. The rule flags every multiplication of two non-constant integer expressions that is (explicitly or implicitly) converted to a larger integer type. The conversion is an indication that the expression would produce a result that would be too large to fit in the smaller integer type.

int i = 2000000000;
long j = i * i; //Wrong: due to overflow on the multiplication between ints, 
                //will result to j being -1651507200, not 4000000000000000000

long k = (long) i * i; //Correct: the multiplication is done on longs instead of ints, 
                       //and will not overflow

long l = static_cast<long>(i) * i; //Correct: modern C++

References

Multiplicative Operators and the Modulus Operator Integer overflow CWE-190 CWE-192 CWE-197 CWE-681

68747470733a2f2f692e6962622e636f2e636f6d2f5942625a773035712f59656c6c6f772d616e642d426c61636b2d436972636c65732d417274732d616e642d4372616674732d43726561746976652d4167656e63792d4c6f676f2e706e67

odaysec avatar May 07 '25 10:05 odaysec

ping @zpostfacto lets merged this pull-request for patch the vulnerable.

odaysec avatar May 07 '25 10:05 odaysec

CUtlMemoryBase::CUtlMemoryBase(int nSizeOfType, int nGrowSize, int nInitAllocationCount)
    : m_pMemory(nullptr),
      m_nAllocationCount(nInitAllocationCount),
      m_nGrowSize(nGrowSize),
      m_unSizeOfElements(nSizeOfType)
{
    if (m_unSizeOfElements <= 0)
        throw std::invalid_argument("Element size must be positive");

    if (m_nGrowSize < 0)
        throw std::invalid_argument("Grow size cannot be negative");

    if (m_nAllocationCount > 0)
    {
        if (static_cast<size_t>(m_nAllocationCount) >
            std::numeric_limits<size_t>::max() / static_cast<size_t>(m_unSizeOfElements))
        {
            throw std::overflow_error("Allocation size overflow");
        }

        size_t totalSize = static_cast<size_t>(m_nAllocationCount) * static_cast<size_t>(m_unSizeOfElements);

        UTLMEMORY_TRACK_ALLOC();
        m_pMemory = PvAlloc(totalSize);

        if (!m_pMemory)
        {
            throw std::bad_alloc();
        }
    }
}

usta avatar May 07 '25 16:05 usta