# Patch Analysis: CivetWeb PUT + SSI #exec RCE

## 1. What the repro's "fix" changes

The reproduction stage used an **artificial control build** rather than an upstream fix:
- **Vulnerable build:** commit `3309a6c` (last working master before a syntax-error commit) compiled with default flags.
- **Fixed/control build:** the *same* commit `3309a6c` compiled with `-DNO_POPEN`.

The `NO_POPEN` flag removes the entire `do_ssi_exec()` function and the SSI `#exec` dispatch from `src/civetweb.c`:

```c
#if !defined(NO_POPEN)
static void
do_ssi_exec(struct mg_connection *conn, char *tag)
{
    ...
    if ((file.access.fp = popen(cmd, "r")) == NULL) {
        ...
    }
    ...
}
#endif
```

and the corresponding call in `send_ssi_file()`:

```c
#if !defined(NO_POPEN)
} else if ((len > 9) && !memcmp(buf + 5, "exec", 4)) {
    do_ssi_exec(conn, buf + 9);
}
#endif
```

So the control "fix" assumption is:
> If the server is built with `NO_POPEN`, SSI `#exec` cannot call `popen()`, therefore the attacker cannot execute commands through an uploaded `.shtml` file.

## 2. Upstream status

There is **no upstream patch** for this issue at the time of analysis:
- `git log` shows the repository HEAD at `3309a6c` (the same commit used as vulnerable).
- The next commit `588860e3` ("Refactor request handling: don't allow chunked encoding and content length") has a syntax error in `get_request()` and does not compile, so it cannot be used as a fixed reference.
- The `SECURITY.md` does not list SSI `#exec` as an out-of-scope feature; it only notes that `src/main.c` is not used by embedded applications and that test/example folders are not security-reviewed. The core `src/civetweb.c` is explicitly described as production code and subject to intensive review.
- The default configuration still sets `ssi_pattern = "**.shtml$|**.shtm$"` and exposes `put_delete_auth_file` as a configuration option, enabling the same trust boundary crossing described in the ticket.

## 3. What the fix does NOT cover

The `NO_POPEN` control is **sufficiently narrow** that it stops the demonstrated attack, but it does not address the underlying design issue: an authenticated user can still write files that are later served by the same process. Any other sink that interprets uploaded file content as executable code remains reachable if the corresponding feature is enabled or compiled in.

Specifically, the `NO_POPEN` control does not cover:

1. **Alternative HTTP framing for the same PUT upload.** The vulnerable `put_file()` calls `forward_body_data()` which handles both `Content-Length` and `Transfer-Encoding: chunked` bodies. A chunked PUT uploads the same malicious `.shtml` content through a different request path, but still reaches the same `do_ssi_exec()` sink.

2. **Alternative WebDAV methods when `enable_webdav` is enabled.** With `enable_webdav yes`, CivetWeb accepts `MOVE` and `COPY` requests on file resources. The request URI is treated as the source, and the `Destination` header supplies the target. An authenticated attacker can upload a benignly-named file (e.g., `pwn.txt`) via PUT, then `MOVE` it to `pwn.shtml`, which matches the default `ssi_pattern`. The GET then triggers `do_ssi_exec()`.

3. **Other SSI-matched extensions.** The default `ssi_pattern` also matches `*.shtm`. The same `popen()` sink is reached regardless of whether the uploaded file ends in `.shtml` or `.shtm`.

4. **Different SSI directives that indirectly reach the same sink.** `<!--#include file="..."-->` recursively calls `send_ssi_file()` on the included file. If the included file matches `ssi_pattern`, its `<!--#exec ...-->` directive is executed. This is a different *data path* inside the SSI engine but still reaches `do_ssi_exec()`.

## 4. What the fix DOES cover

- Any SSI directive of the form `<!--#exec "command"-->` inside a file matching `ssi_pattern` is ignored when `NO_POPEN` is defined.
- The GET response for an uploaded `.shtml` containing only `#exec` returns no command output, because the `popen()` branch is compiled out.

## 5. Comparison: behavior before and after the fix

| Input | Vulnerable build (`3309a6c`) | Fixed/control build (`3309a6c` + `NO_POPEN`) |
|-------|-------------------------------|---------------------------------------------|
| `PUT /pwn.shtml` with `<!--#exec "id; uname -a" -->` | 200 OK, file stored | 200 OK, file stored |
| `GET /pwn.shtml` | 200 OK with `id`/`uname` output in body | 200 OK, empty body (no command output) |
| `PUT /pwn.shtml` with `Transfer-Encoding: chunked` | same RCE | same blocking (no command output) |
| `PUT /pwn.txt` then `MOVE /pwn.txt` → `/pwn.shtml` (WebDAV) | same RCE | same blocking (no command output) |
| `PUT /pwn.shtm` with `#exec` | same RCE | same blocking (no command output) |

## 6. Is the fix complete?

The `NO_POPEN` control is **complete against the specific SSI `#exec` RCE sink** but it is **not a general fix for the trust boundary issue**. A real patch should address at least one of the following:
- Decouple the file-upload capability from the SSI execution capability (e.g., require an explicit option to enable SSI `#exec`, or disable it by default).
- Restrict `ssi_pattern` to paths that cannot be created by `PUT` uploads.
- Add a config option to disable `#exec` independently from `#include`.
- Apply `is_in_script_path` or similar checks to `do_ssi_include` to prevent path traversal via `file=`/`virtual=`.

Because the current upstream repository still contains the vulnerable `popen()` code path and no real commit fixes it, the variant stage is effectively testing alternate triggers against a hypothetical control build. No true bypass of the `NO_POPEN` control was found, because the only remaining code-execution sink reachable from an uploaded file is the `do_ssi_exec()` function that the control removes.