Skip to content

Commit 4aa96d6

Browse files
authored
Fix OOB read/write in interopserver POST filename parsing (#6055)
## Description ## Summary - `HttpRequest::ReceiveUniDiData` called `strstr(FileName, "\r\n")` on a raw `QUIC_BUFFER` from `Event->RECEIVE.Buffers`, which is not NUL-terminated. A peer sending `POST ` followed by non-CRLF bytes with no NUL byte in the first delivered buffer caused `strstr` to walk past `FirstBuffer->Buffer + FirstBuffer->Length` into adjacent memory, and the subsequent `*FileNameEnd = '\0'` wrote a NUL byte outside the receive buffer - Replaced `strstr` with a bounded scan limited to `FirstBuffer->Length - 5`, and copy the filename into a local NUL-terminated buffer instead of mutating the receive buffer. Also added an explicit length cap so the filename can't silently truncate inside the later `snprintf`. ## Testing NA ## Documentation NA
1 parent 1c98961 commit 4aa96d6

2 files changed

Lines changed: 12 additions & 4 deletions

File tree

src/tools/interopserver/InteropServer.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
#include "InteropServer.h"
1313

14+
#include <algorithm>
15+
1416
const QUIC_API_TABLE* MsQuic;
1517
HQUIC Configuration;
1618
const char* RootFolderPath;
@@ -313,13 +315,18 @@ HttpRequest::ReceiveUniDiData(
313315
return false;
314316
}
315317

316-
char* FileName = (char*)FirstBuffer->Buffer + 5;
318+
char HeaderBuf[256];
319+
const size_t CopyLen = std::min((size_t)FirstBuffer->Length, sizeof(HeaderBuf) - 1);
320+
memcpy(HeaderBuf, FirstBuffer->Buffer, CopyLen);
321+
HeaderBuf[CopyLen] = '\0';
322+
323+
char* FileName = HeaderBuf + 5;
317324
char* FileNameEnd = strstr(FileName, "\r\n");
318325
if (FileNameEnd == nullptr) {
319326
printf("[%s] Invalid post suffix\n", GetRemoteAddr(MsQuic, QuicStream).Address);
320327
return false;
321328
}
322-
*FileNameEnd = '\0'; // We shouldn't be writing to the buffer. Don't imitate this.
329+
*FileNameEnd = '\0';
323330

324331
if (strstr(FileName, "..") != nullptr) {
325332
printf("[%s] '..' found\n", GetRemoteAddr(MsQuic, QuicStream).Address);
@@ -339,8 +346,7 @@ HttpRequest::ReceiveUniDiData(
339346
return false;
340347
}
341348

342-
FileNameEnd += 2; // Skip "\r\n"
343-
SkipLength = (uint32_t)((uint8_t*)FileNameEnd - FirstBuffer->Buffer);
349+
SkipLength = (uint32_t)((FileNameEnd - HeaderBuf) + 2);
344350
}
345351

346352
for (uint32_t i = 0; i < BufferCount; ++i) {

src/tools/interopserver/InteropServer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
#define _CRT_SECURE_NO_WARNINGS 1
99

10+
#define NOMINMAX
11+
1012
#define QUIC_API_ENABLE_PREVIEW_FEATURES
1113

1214
#include "msquichelper.h"

0 commit comments

Comments
 (0)