open
https://gitlab.synchro.net/main/sbbs/-/issues/1205
## Summary
MIME attachments in a **nested** multipart message are never detected or decoded. The Terminal Server therefore never offers them for download, and `smb_getplaintext()` can fail on the same messages — dumping the entire raw MIME body (base64 and all) at the user instead of the readable text.
This is **not** #485. That issue's example body is a flat `multipart/mixed` whose part carries `Content-Disposition: inline`, which
`mime_getattachment()` deliberately skips (`smbtxt.c`, since `c87b2f2723`, 2017-11-26). The bug reported here has a different root cause and reproduces with `Content-Disposition: attachment`.
## Root cause
In `mime_getpart()` (`src/smblib/smbtxt.c`) the `boundary=` parameter value was terminated only by `;` or a closing `"` — never by the end of the header line:
```c
SAFEPRINTF(boundary, "--%s", p);
if ((p = strchr(boundary, quoted_boundary ? '"' : ';')) != NULL)
*p = 0;
```
For the **top-level** `Content-Type` that is harmless: it comes from a NUL-terminated header field. But a **nested** part's `Content-Type` is a pointer into the raw message body, so when `boundary=` is the last parameter on its (folded) line — which is what iOS Mail emits — everything following it is
swallowed into the boundary string.
Observed on a real message, instrumented:
```
[depth=0 boundary=49 '--Apple-Mail-7DFD27B7-7575-40B2-A7CB-EC46A0B9F597'] [depth=1 boundary=162 '--Apple-Mail-0E69CC22-...D9FE\r\nContent-Transfer-Encoding: 7bit\r\n\r\n\r\n--Apple-Mail-0E69CC22-...\r\nContent-Type: text/html']
```
The 162-byte string never matches, so the nested part yields no attachments and no plain text.
## Reproduction
iOS Mail (`X-Mailer: iPhone Mail`) sends a PDF as:
```
multipart/alternative <- top-level Content-Type header field ├── text/plain
└── multipart/mixed <- parsed out of the body
├── text/html
├── application/pdf Content-Disposition: attachment
├── text/html
├── application/pdf Content-Disposition: attachment
└── text/html
```
where the inner container's header is folded:
```
Content-Type: multipart/mixed;
boundary=Apple-Mail-0E69CC22-2E19-4166-95C0-41394145D9FE Content-Transfer-Encoding: 7bit
```
Minimal reduction — two bodies identical except for one character. With `boundary=INNER` the attachment is invisible; with `boundary=INNER; x=1` it is found. That single `;` is the whole bug.
## Fix
Terminate the boundary value at the end of the header line as well, and bound the parse to the part's own (folded) `Content-Type` field:
```c
content_type += len;
/* A nested part's Content-Type is parsed out of the message body rather than
* from a header field of its own, so it is only terminated by the end of the
* (possibly folded) header line. */
const char* ct_end = content_type;
while ((ct_end = strstr(ct_end, "\r\n")) != NULL) {
if (ct_end[2] != ' ' && ct_end[2] != '\t') /* not a folded continuation */
break;
ct_end += 2;
}
if (ct_end == NULL)
ct_end = content_type + strlen(content_type);
p = strcasestr(content_type, "boundary=");
if (p == NULL || p >= ct_end)
return NULL;
p += 9;
while (*p == ' ' || *p == '\t') /* RFC822 white space is allowed around the '=' */
p++;
if (*p == '"') {
p++;
quoted_boundary = true;
}
SAFEPRINTF(boundary, "--%s", p);
p = boundary + 2;
FIND_CHARSET(p, quoted_boundary ? "\"\r\n" : "; \t\r\n");
*p = 0;
truncsp(boundary); // RFC2046: "NOT ending with white space"
if (boundary[2] == '\0') /* An empty boundary would match every delimiter */
return NULL;
```
Three parts of that are load-bearing, and the last two are needed **because of**
the first:
- **`ct_end` bound.** Once CRLF terminates the value, an unbounded
`strcasestr(content_type, "boundary=")` will happily pick up a `boundary=`
that occurs later in the *message text* (someone quoting a MIME header in an
email) when the part's own `Content-Type` has no boundary parameter. Verified:
without the bound, a body whose text contains `boundary=INNER` at end-of-line
causes a nested `secret.pdf` to be parsed out and offered for download.
- **Empty-boundary rejection.** A malformed `boundary=` with no value yields the
boundary string `--`, which matches the leading dashes of every delimiter *and*
any `--` in the message text. Verified: without this check, a body whose
`text/plain` part ends with the conventional `-- ` signature separator is
silently truncated there.
- **Leading/trailing white space.** `boundary= INNER` (RFC822 permits white space
around the `=`) and `boundary=INNER x=1` are now parsed correctly; an unquoted
boundary is an RFC 2046 token and cannot contain white space.
## Validation
Synthetic matrix, exercising `smb_getattachment()` / `smb_getplaintext()` directly against a nested body:
| inner `boundary` parameter | before | after |
|---|---|---|
| `boundary=INNER` (the reported shape) | not found | **found** |
| `boundary=INNER; x=1` | found | found |
| `boundary="INNER"` / `boundary="INNER"; x=1` | found | found |
| `boundary=` (empty) | not found | not found *(and text no longer truncated)* |
| `boundary= INNER` | not found | **found** |
| `boundary="INNER` (no closing quote) | not found | **found** |
| `boundary=INNER x=1` | not found | **found** |
| `x=1; boundary=INNER` | not found | **found** |
| no `boundary` parameter at all | not found | not found |
| `boundary=` present only in message *text* | not found | not found |
The reported message now yields both PDFs, decoding to valid complete 8-page documents (`%PDF-1.3` header, intact `%%EOF` trailer, 970,301 and 276,876 bytes).
Swept the **entire mail base (8,173 messages)**, comparing attachment count, plain-text length and decoded subtype/charset before and after:
- attachments found: **60 -> 76** across 13 messages
- messages whose body is now MIME-decoded rather than dumped raw: **11**,
removing **6.96 MB** of base64 that was previously written to the terminal
- messages that lost an attachment: **none**
- messages that lost their plain-text decode, or whose plain text got shorter:
**none**
Compiles clean under GCC and Clang (`-Wall -Wextra`) and via the project's own `make -C src/smblib lib`; the edited hunk is uncrustify-clean.
Note: this is library-level validation. An end-to-end Terminal Server read of such a message has not been exercised.
## Known limitation (not addressed here)
`smb_getattachment()`'s `index` argument does not enumerate across *sibling* nested containers: the `found` counter in `mime_getpart()` is per-recursion- level, so with two sibling `multipart/mixed` branches holding one attachment each, index 1 returns nothing and the second attachment is unreachable. Multiple attachments within a *single* nested container (the common case, and the reported message) enumerate correctly. This was unreachable before, since nested containers were never parsed at all. Worth a follow-up.
— *Authored by Claude (Claude Code), on behalf of @rswindell*
---
ï¿ Synchronet ï¿ Vertrauen ï¿ Home of Synchronet ï¿ [vert/cvs/bbs].synchro.net