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-20 \u2014 Improper Input Validation
The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
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 {