Skip to content

🔒 Fix CRLF injection vulnerabilities (backports #657, #658, #659, #660, #636, #661)#662

Merged
nevans merged 7 commits intov0.5-stablefrom
backport/v0.5/raw_data-warnings
Apr 23, 2026
Merged

🔒 Fix CRLF injection vulnerabilities (backports #657, #658, #659, #660, #636, #661)#662
nevans merged 7 commits intov0.5-stablefrom
backport/v0.5/raw_data-warnings

Conversation

@nevans
Copy link
Copy Markdown
Collaborator

@nevans nevans commented Apr 23, 2026

This backports the fixes for various CRLF injection vulnerabilities to v0.5-stable.

Important

This fixes a CSRF/command injection vulnerability for Symbol arguments.

Important

This fixes a CSRF/command injection vulnerability for the attr argument to #store/#uid_store.

Important

This fixes a CSRF/command injection vulnerability for the storage_limit argument to #setquota.

Important

This fixes the CRLF injection / command injection vulnerability in RawData, which is used for the criteria argument #search/#uid_search when it is a String, and the attr argument to #fetch/#uid_fetch when is a String, or the String members of attr when it is an Array.

Caution

Please note: RawData can still be used for other forms of argument injection!

The following documentation PRs were also backported:

nevans added 7 commits April 23, 2026 10:44
#657]

Flags should not allow `atom-specials`.

Previously, no validation was done on symbol data.  Sending atom or flag
args which contain atom specials could lead to various errors.

Although this could theoretically include injection attacks, this is not
considered to be a critical vulnerability in `net-imap`, for the
following reason:  Valid "system flag" inputs are restricted to an
enumerated set of RFC-defined flag types.  User-defined "keyword" flags
are sent as atoms, not flags, which use string inputs (strings which
can't be sent as an atom will be quoted or sent as a literal).  `\Seen`
as a flag (symbol argument) is semantically different from `Seen` as a
keyword (string argument).  So there is no scenario where it is
appropriate to call `#to_sym` on unvetted user input.  Any code which
calls `#to_sym` indiscriminately on user-input is already buggy.

Nevertheless, users should reasonably be able to rely on `net-imap` to
do very basic input validation on its basic input types.
…rts #658]

Previously, `#store` and `#uid_store` wrapped `attr` with `RawData`.
But that's completely unnecessary.  `+`, `-`, and `.` are atom chars,
and every STORE "message data item" defined in any RFC is an `atom`:

```
 FLAGS      FLAGS.SILENT
+FLAGS     +FLAGS.SILENT
-FLAGS     -FLAGS.SILENT
ANNOTATION ANNOTATION.SILENT
```

We can revisit this in the future, if some new extension uses a non-atom
for its STORE "message data item", but that seems unlikely.

Note also that `Atom` is only applied to `String` arguments.
…kports #636]

This updates the QUOTA docs with references to RFC9208 (`QUOTA=RES-*`).
For the most part, RFC9208 is backward compatible with RFC2087.  So this
updates the documentation to reference it wherever relevant.

This also documents how `net-imap`'s support for `QUOTA` is incomplete:
that it only supports setting `STORAGE` and can only parse a single
resource type (which is _usually_ `STORAGE`).

The RFCs are very clear that `quota root` is _not_ the same as
`mailbox`, so this updates method parameters and rdoc to reflect that.
`MailboxQuota#mailbox` is incorrectly named, but renaming it would be
backward incompatible.  This just updates the documentation and adds
`quota_root` as an alias.

This also fixes the rdoc for `MailboxQuota#quota` to specify that it is
the _storage_ limit, rather than any other quota resource type.  This
adds a note in the rdoc that _only_ `STORAGE` is currently supported.

`GETQUOTA` generally _is_ available to normal users.  That comment on
`#getquota` may have been copied from `SETQUOTA`.

See also: #622
…rts #659]

There's no reason to use `RawData` for this.  We can use Net::IMAP's
standard command argument formatting to send a parenthesized list.
…kports #660]

This parses a RawData string into an array of `text`, `literal`, and
`literal8` parts.  This fixes embedded literals so they correctly wait
for server continuation request before sending.  Non-synchronizing
literals are also parsed correctly.

This adds `Net::IMAP::RawText` which sends verbatim (like `RawData` did
previously), and handles `text` validations:
* `text` can't contain CR, LF, or NULL
* `text` must be ASCII compatible or valid UTF-8

The existing `Literal` and `Literal8` classes handle literal validation:
* `literal` can't contain NULL byte, but `literal8` can

Additionally, `RawData` validates that:
* embedded literal bytesize must be <= remaining string bytesize
* final `text` cannot end with `{number}` (in case a `CRLF` comes after)

This does _not_ make RawData arguments safe from every type of injection
attack.  However, without losing any significant flexibility, this
_does_ prevent unescaped `CRLF` from creating a _command_ injection.
#661]

Now that `setquota`, `store`, and `uid_store` have been fixed, there
should only be two parameters that still use `RawData`: search
`criteria` and fetch `attr` (and the `UID` variants).

`#search` criteria (when a string) had already been documented, but this
aspect of `#fetch` attr was _not_ previously documented!
@nevans nevans added bug Something isn't working documentation Improvements or additions to documentation backport This issue or PR is for a stable release branch security vulnerability patch Pull requests that address security vulnerabilities labels Apr 23, 2026
@nevans nevans merged commit 6bf02ae into v0.5-stable Apr 23, 2026
46 checks passed
@nevans nevans deleted the backport/v0.5/raw_data-warnings branch April 23, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This issue or PR is for a stable release branch bug Something isn't working documentation Improvements or additions to documentation security vulnerability patch Pull requests that address security vulnerabilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant