diff --git a/ext/naett-lib/README-ppsspp.md b/ext/naett-lib/README-ppsspp.md index 114b630b61..2ea0e361a2 100644 --- a/ext/naett-lib/README-ppsspp.md +++ b/ext/naett-lib/README-ppsspp.md @@ -33,3 +33,13 @@ Keep this list up to date - it's what makes it possible to move to a newer upstr - `src/naett_linux.c`: `curl_easy_setopt` is varargs and takes a `long` for these options; upstream passed `int` literals and `int` variables, which is UB on LP64 (and what curl's own typecheck macros warn about). They're `1L`/`(long)` now. +- `src/naett_core.c`: `naettFree` never freed `options.userAgent`, though it's `strdup`'d by + the same setter as `method`. Leaked once per request for every caller that sets a user agent, + which we do on all of them. +- `src/naett_core.c`: `defaultBodyWriter` doubled an `int` capacity until it fit, which is signed + overflow on a large response, and used the `realloc` result without checking it - losing the + old pointer and then `memcpy`ing through NULL. Grows in `int64_t` against `INT_MAX` and + reports failure by returning short, which every caller already treats as an error. +- `src/naett_linux.c`: `headerCallback` only handed its `strndup` to the header list when the + line had a colon, and leaked it otherwise. curl passes the status line and the blank line that + ends the header block, so that leaked at least twice per response. diff --git a/ext/naett-lib/src/naett_core.c b/ext/naett-lib/src/naett_core.c index 0e9ee529e7..bc6e2e24e1 100644 --- a/ext/naett-lib/src/naett_core.c +++ b/ext/naett-lib/src/naett_core.c @@ -1,6 +1,8 @@ #include "naett_internal.h" #include #include +#include +#include #include #include #include @@ -81,18 +83,34 @@ static int defaultBodyReader(void* dest, int bufferSize, void* userData) { return bytesToRead; } +// PPSSPP: `bytes` and the resulting capacity come off the network, so the growth has to be +// done in a type that can't wrap into a negative capacity, and a failed realloc has to leave +// the buffer we already have intact. Returning short is how this reports failure - every +// caller compares the result against what it asked to write and errors the request out. static int defaultBodyWriter(const void* source, int bytes, void* userData) { Buffer* buffer = (Buffer*) userData; - int newCapacity = buffer->capacity; - if (newCapacity == 0) { - newCapacity = bytes; + if (bytes <= 0) { + return 0; } - while (newCapacity - buffer->size < bytes) { - newCapacity *= 2; - } - if (newCapacity != buffer->capacity) { - buffer->data = realloc(buffer->data, newCapacity); - buffer->capacity = newCapacity; + if (buffer->capacity - buffer->size < bytes) { + int64_t newCapacity = buffer->capacity > 0 ? buffer->capacity : bytes; + while (newCapacity - buffer->size < bytes) { + newCapacity *= 2; + if (newCapacity > INT_MAX) { + newCapacity = INT_MAX; + break; + } + } + if (newCapacity - buffer->size < bytes) { + // Doesn't fit in the int sizes this struct uses. + return 0; + } + void* newData = realloc(buffer->data, (size_t)newCapacity); + if (newData == NULL) { + return 0; + } + buffer->data = newData; + buffer->capacity = (int)newCapacity; } char* dest = ((char*)buffer->data) + buffer->size; memcpy(dest, source, bytes); @@ -388,6 +406,8 @@ void naettFree(naettReq* request) { KVLink* node = req->options.headers; freeKVList(node); free((void*)req->options.method); + // PPSSPP: userAgent is strdup'd by stringSetter like method is, and was never freed. + free((void*)req->options.userAgent); free((void*)req->url); free(request); } diff --git a/ext/naett-lib/src/naett_linux.c b/ext/naett-lib/src/naett_linux.c index 165abb1960..3cdbac25f6 100644 --- a/ext/naett-lib/src/naett_linux.c +++ b/ext/naett-lib/src/naett_linux.c @@ -171,6 +171,9 @@ static size_t headerCallback(char* buffer, size_t size, size_t nitems, void* use size_t headerSize = size * nitems; char* headerName = strndup(buffer, headerSize); + if (headerName == NULL) { + return headerSize; + } char* split = strchr(headerName, ':'); if (split) { *split = 0; @@ -195,6 +198,11 @@ static size_t headerCallback(char* buffer, size_t size, size_t nitems, void* use node->key = headerName; node->value = headerValue; res->headers = node; + } else { + // PPSSPP: no colon, so the list never takes ownership of this copy. curl hands us the + // status line and the blank line that terminates the header block, neither of which has + // one - so upstream leaked at least twice per response, more with redirects. + free(headerName); } return headerSize;