Integer underflow → multi-GB allocation (DoS) in EC tag deserialization #877

File: src/libs/ec/cpp/ECTag.cpp, CECTag::ReadFromSocket(), line 441
Severity: Medium–High (reachable pre-authentication on any reachable EC socket)
Type: Unsigned integer underflow leading to an attacker-controlled oversized new[]

Description

When deserializing a tag from the External Connection (EC) protocol, the payload
length is computed by subtracting the serialized size of the (already-read) child
tags from the wire-supplied total length:

unsigned int tmp_len = m_dataLen;       // wire-supplied, attacker-controlled (read at line 430)
m_dataLen = 0;
const bool useLargeCount = (socket.m_rx_flags & EC_FLAG_LARGE_TAG_COUNT) != 0;
m_dataLen = tmp_len - GetTagLen(useLargeCount);   // line 441 — unsigned subtraction
if (m_dataLen > 0) {
    NewData();                          // ECTag.h:245 -> m_tagData = new char[m_dataLen];
    if (!socket.ReadBuffer(m_tagData, m_dataLen)) {
        return false;
    }
}

m_dataLen is ec_taglen_t (unsigned 32-bit). tmp_len is read directly from
the wire at line 430, before the child tags are parsed. The children are then
parsed by ReadChildren() according to their own length fields, independently of
tmp_len. GetTagLen() afterwards returns the serialized size of those children.

A malicious peer can therefore send a tag whose declared tmp_len is smaller
than the real serialized size of its children. The subtraction at line 441 then
underflows to a value close to 0xFFFFFFFF (~4 GB). The if (m_dataLen > 0)
guard passes, and NewData() performs new char[m_dataLen] with that huge size.

Reachability — why the packet-size cap does not protect against this

ReadHeader() (src/libs/ec/cpp/ECSocket.cpp:577-584) caps the whole packet at
16 MB pre-auth / 256 MB post-auth. That cap bounds the receive buffer, not
m_dataLen: the underflow is per-tag arithmetic and is independent of the packet
size, so a perfectly valid sub-cap packet can still drive a ~4 GB allocation.
Tags are parsed while reading the authentication request itself, so this is
reachable before authentication by any peer able to open an EC connection
(network exposure depends on the configured ECAddress; the default is
localhost).

Impact

Memory-exhaustion / std::bad_alloc denial of service of the aMule daemon from a
single malformed EC packet. ReadBuffer() later bounds the actual read against
the available buffer, so this is not an out-of-bounds read — the damage is the
oversized allocation itself.

Suggested fix

Validate tmp_len >= GetTagLen(useLargeCount) before the subtraction and reject
the tag (return false) otherwise, so m_dataLen cannot underflow. (A related
reader-side over-count was already partially addressed for #199 via the
GetTagLen iteration cap; this is the complementary missing lower-bound check.)