CVE-2020-13111 (Other, published 2020-05-16T15:15Z)
Description: value: NaviServer 4.99.4 to 4.99.19 allows denial of service due to the nsd/driver.c ChunkedDecode function not properly validating the length of a chunk. A remote attacker can craft a chunked-transfer request that will result in a negative value being passed to memmove via the size parameter, causing the process to crash.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-787 \u2014 Out-of-bounds Write
The product writes data past the end, or before the beginning, of the intended buffer.
Repository: https://bitbucket.org/naviserver/naviserver
Files changed:
nsd/driver.c
tests/http_chunked.test
Commit message: fix for bug https://sourceforge.net/p/naviserver/bugs/89/

A negative value provided as chunk encoding length led to a potential crash.
A test case for this case was added to the regression test.


Vulnerable code:
static bool ChunkedDecode(Request *reqPtr, bool update)
 *      NS_TRUE when chunk was complete, NS_FALSE otherwise
 *      updates the buffer if update is true (and adjusts reqPtr->chunkWriteOff)
 *      updates always reqPtr->chunkStartOff to allow incremental operations
static bool
    bool              success = NS_TRUE;
        size_t  chunk_length;
            success = NS_FALSE;
        chunk_length = (size_t)strtol(chunkStart, NULL, 16);
        if (p + 2 + chunk_length > end) {
            success = NS_FALSE;
            memmove(writeBuffer, p + 2, chunk_length);
            reqPtr->chunkWriteOff += chunk_length;
            *(writeBuffer + chunk_length) = '\0';
        reqPtr->chunkStartOff += (size_t)(p - chunkStart) + 4u + chunk_length;
    return success;
    Ns_Log(Dev, "=== length < avail (length %" PRIuz
        bool   complete;
        size_t currentContentLength;
        complete = ChunkedDecode(reqPtr, NS_TRUE);
        if ((!complete)
test http_chunked-3-1 {

Fixed code:
static SockState ChunkedDecode(Request *reqPtr, bool update)
 *      SOCK_READY when chunk was complete, SOCK_MORE when more data is
 *      requried, or some error condition.
 *      Updates the buffer if update is true (and adjusts
 *      reqPtr->chunkWriteOff).  Updates always reqPtr->chunkStartOff to allow
 *      incremental operations.
static SockState
    SockState         result = SOCK_READY;
        long    chunkLength;
            result = SOCK_MORE;
        chunkLength = strtol(chunkStart, NULL, 16);
        *p = '\r';
        if (chunkLength < 0) {
            Ns_Log(Warning, "ChunkedDecode: negative chunk length");
            result = SOCK_BADREQUEST;
            break;
        }
        if (p + 2 + chunkLength > end) {
            result = SOCK_MORE;
            memmove(writeBuffer, p + 2, (size_t)chunkLength);
            reqPtr->chunkWriteOff += (size_t)chunkLength;
            *(writeBuffer + chunkLength) = '\0';
        reqPtr->chunkStartOff += (size_t)(p - chunkStart) + 4u + (size_t)chunkLength;
    return result;
    Ns_Log(Debug, "=== length < avail (length %" PRIuz
        SockState chunkState;
        size_t    currentContentLength;
        chunkState = ChunkedDecode(reqPtr, NS_TRUE);
        if ((chunkState == SOCK_MORE)

        } else if (chunkState != SOCK_READY) {
            return chunkState;
test http_chunked-2.4 {
    POST request with invalid chunked content

} -constraints serverListen -setup {
    ns_register_proc PUT /tclchunked {
        set content [ns_conn content]
        #ns_log notice "PUT received <$content>"
        ns_return 200 text/plain $content
    }
} -body {
    nstest::http-0.9 -http 1.1 \
        -omitcontentlength 1 \
        -setheaders {Transfer-Encoding Chunked} \
        -getheaders {Transfer-Encoding Content-Length} -getbody 1 \
        PUT /tclchunked "-1\r\nchunk\r\n0\r\n\r\n"
} -cleanup {
    ns_unregister_op PUT /tclchunked
} -match glob -result {200 {} 0 {HTTP/1.0 400 Bad Request*}}

test http_chunked-3.1 {