Reject CR/LF/NUL in user-supplied IMAP command text#119
Draft
mpscholten wants to merge 5 commits into
Draft
Conversation
Adds UIDPLUS extension support so callers can recover the UIDs the server assigns on APPEND/COPY and target expunges by UID: - appendFullUID: like appendFull, returns the APPENDUID response code - copyUID / copyUIDs / copyUIDR: UID COPY returning the COPYUID code - uidExpunge / uidExpungeR: UID EXPUNGE over a UID set or range New types AppendUID and CopyUID, the UIDSet alias, and the APPENDUID/COPYUID/UIDNOTSTICKY status codes with their parsers. sendCommandWithResponse exposes the tagged ServerResponse so the response codes can be read. The existing appendFull/copyFull keep their old signatures by discarding the UID result. Covered by new parser cases in baseTest and a dedicated imapUIDPlusTest group exercising the API against scripted server responses.
RFC 3501 keywords, response codes, flag names and status attributes are case-insensitive, but the parser matched them with case-sensitive `string`, so any server replying with non-canonical casing (e.g. `ok`, `* search`, `[uidvalidity ...]`, `\seen`) caused a parse error. Adds `stringCI`/`charCI` and applies them throughout the response parser. Along the way the tagged/fatal response handling is factored into `pWithTaggedOrFatal`, `pDone` is split into `pRespCode`/`pRespText`/`pStatusCode`, and string parsing is unified (`pQuotedString`/`pLiteralString`/`pAString`/`pMailboxName`). This also: - surfaces an untagged `* BYE` as a fatal response instead of failing - accepts an empty `* SEARCH` reply (no matches) - accepts a `NIL` hierarchy separator in LIST/LSUB - stops `atomChar` from running past CR/LF Covered by a new caseInsensitiveTest group.
IMAP SEARCH string keys (BCC/BODY/CC/FROM/HEADER/SUBJECT/TEXT/TO/
X-GM-RAW) were interpolated unquoted, so any value containing a space
or special character produced a malformed command. They are now wrapped
with quoteIMAPString.
LARGER/SMALLER wrongly wrapped their octet count in `{}` (the literal
syntax), e.g. `LARGER {100}`; the size is a plain number (`LARGER 100`).
SEARCH now auto-detects non-ASCII text in a query and prepends
`CHARSET UTF-8`, and command bytes are sent UTF-8 encoded
(sendCommandNoResponse) so the non-ASCII bytes survive instead of being
truncated to Latin-1.
Covered by a new imapSearchApiTest group.
A handful of independent fixes to the IMAP client:
- connectStream accepts a PREAUTH greeting and matches the greeting
status case-insensitively (was: only exact "* OK")
- AUTHENTICATE now loops over '+' challenge continuations via
getAuthResponse, so multi-step SASL mechanisms (e.g. XOAUTH2) that
emit an error continuation before the tagged reply complete instead
of stalling; the challenge prefix is matched as "+" not "+ "
- getResponse understands non-synchronising literals ({n+}) and the
binary literal prefix (~{n}), not just plain {n}
- fetch/fetchPeek/fetchR/fetchRPeek normalize a NIL body to empty
- storeFull reads result flags from the FLAGS key (was "FLAG", which
never matched) and quotes X-GM-LABELS values
- escapeLogin is unified with quoteIMAPString (drops the invalid
backslash-escaping of { and } inside a quoted string)
Covered by a new imapRobustnessTest group.
User-controlled strings were interpolated straight into command lines. A value containing CR, LF or NUL could therefore terminate the current command and smuggle additional IMAP commands onto the connection (command injection). Adds validateCommandText, which fails loudly when such a character is present, and wires it into every user-supplied argument: login credentials, mailbox names (select/examine/create/delete/rename/ subscribe/unsubscribe/status/append/copy/move), SEARCH charset and string query terms, FETCH commands and header field names, STORE flag keywords, and Gmail labels. Covered by a new imapHardeningTest group.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Top of the stack — based on #118 → #117 → #116 → #115. Final PR in the series upstreaming the belege.ai fork's IMAP fixes. The diff shows the earlier commits until they merge; the commit for this PR is "Reject CR/LF/NUL in user-supplied IMAP command text". Merge order: #115 → #116 → #117 → #118 → this.
What
User-controlled strings are interpolated straight into IMAP command lines. A value containing CR, LF or NUL can terminate the current command and smuggle additional commands onto the connection — i.e. IMAP command injection (e.g. a mailbox name
INBOX\r\nA001 DELETE victim).validateCommandTextfails loudly when such a character is present, and is wired into every user-supplied argument:select/examine/create/delete/rename/subscribe/unsubscribe/status/append/copy/moveSEARCHcharset and string query terms (BCC/BODY/CC/FROM/HEADER/SUBJECT/TEXT/TO/X-GM-RAW, recursing throughNOT/OR)FETCHcommands and header field namesSTOREflag keywords and Gmail labels (appendFullflags too)(UID-set arguments are excluded — they're always generated internally from
[UID]/ranges and only ever contain digits,,,:.)Tests
New
imapHardeningTest: injection attempts via login, select, search, fetch and store are all rejected with aCR, LF, or NULerror.🤖 Generated with Claude Code