Skip to content

馃 Validate ID values contain only valid bytes - #698

Merged
nevans merged 3 commits into
v0.6.4-patchesfrom
security/validate-quoted-data
Jun 9, 2026
Merged

馃 Validate ID values contain only valid bytes#698
nevans merged 3 commits into
v0.6.4-patchesfrom
security/validate-quoted-data

Conversation

@nevans

@nevans nevans commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

This shares most of the validation implementation with RawText, via an extracted shared superclass.

In currently released versions of net-imap, QuotedString is not used to quote regular string arguments. It's currently only used by Net::IMAP#id (the ID extension). This PR reverses the dependency, so that send_string_data calls QuotedString, rather than vice versa.

Without this patch, Net::IMAP#id is vulnerable to the same type of CRLF injection issues as in CVE-2026-42258 (GHSA-75xq-5h9v-w6px) and CVE-2026-42257 (GHSA-hm49-wcqc-g2xg). The string will be quoted, which imposes some limits on what command arguments can be injected, but many destructive commands will work fine without any quoted specials.

Presumably, Net::IMAP#id should be very unlikely to be called with untrusted input, making this less likely to be exploitable. Nevertheless, CRLF injection should be prevented!

nevans added 3 commits June 8, 2026 11:33
Also update internal RawText docs.
This shares most of the validation implementation with RawText, via an
extracted shared superclass.

Please note: in currently released `net-imap`, QuotedString is _not_
used to quote regular string arguments.  It's currently only used by
`Net::IMAP#id` (the `ID` extension).

Without this patch, `Net::IMAP#id` is vulnerable to the same CRLF
injection issues in GHSA-75xq-5h9v-w6px and GHSA-hm49-wcqc-g2xg.  The
string will be quoted, which may limit the ability to inject some
commands.  Presumably, `Net::IMAP#id` is unlikely to be called with
user-provided input, making this less likely to be exploitable.
Nevertheless, CRLF injection should be prevented!
This removes the duplicated string quoting implementation.  It also
introduces stricter enforcement of `quoted` string validation..
@nevans nevans added bug Something isn't working security vulnerability patch Pull requests that address security vulnerabilities labels Jun 9, 2026
@nevans
nevans changed the base branch from master to v0.6.4-patches June 9, 2026 14:34
@nevans
nevans merged commit b9d1972 into v0.6.4-patches Jun 9, 2026
39 checks passed
@nevans
nevans deleted the security/validate-quoted-data branch June 9, 2026 14:35
@nevans nevans added the backport-0.5 This ticket needs to be backported to the v0.5-stable branch. label Jun 9, 2026
nevans added a commit that referenced this pull request Jul 2, 2026
nevans added a commit that referenced this pull request Jul 2, 2026
This shares most of the validation implementation with RawText, via an
extracted shared superclass.

Please note: in currently released `net-imap`, QuotedString is _not_
used to quote regular string arguments.  It's currently only used by
`Net::IMAP#id` (the `ID` extension).

Without this patch, `Net::IMAP#id` is vulnerable to the same CRLF
injection issues in GHSA-75xq-5h9v-w6px and GHSA-hm49-wcqc-g2xg.  The
string will be quoted, which may limit the ability to inject some
commands.  Presumably, `Net::IMAP#id` is unlikely to be called with
user-provided input, making this less likely to be exploitable.
Nevertheless, CRLF injection should be prevented!
nevans added a commit that referenced this pull request Jul 23, 2026
This bug was introduced by a742a10.  When I extracted the
`UNQUOTABLE_CHARS` regexp into a constant, I didn't copied it exactly.
It was converted from a character class into a sequence of bytes:
```ruby
    # It should've been this:
    UNQUOTABLE_CHARS = /[\0\r\n]/n
    # But I incorrectly copied it as this:
    UNQUOTABLE_CHARS = /\0\r\n/n
```

If that regexp were the only guard, this regexp would've allowed CRLF
injections!  Fortunately, quoted strings are now sent using
`Net::IMAP::QuotedString` (since #698, released in v0.6.4.1), and that
_does_ correctly validate that the input is a valid quoted string.  So,
the string isn't _sent_ as invalid quoted data.

Unfortunately, this does result in an exception for strings that could
be valid literals.  And, because it crashes while sending command
arguments, rather than during validation, the connection will be broken.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-0.5 This ticket needs to be backported to the v0.5-stable branch. bug Something isn't working security vulnerability patch Pull requests that address security vulnerabilities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant