Backport the security fixes to v0.4 for a 0.4.25 release#707
Conversation
`Atom` and `Flag` have only been used for argument validation since v0.6.4 (as well as v0.5.14 and v0.4.24), and they validated for absense of `atom-specials`. But they failed to check that the strings are not empty. While this could be used to create syntax errors, I don't believe it amounts a security vulnerability. The result would be no different from any other `BAD` server response, which an application must be prepared to handle.
Previously, integer arguments had to be 32-bit unsigned, and a special case exception was made for the number in a `["CHANGEDSINCE", number]` array. But several other command arguments can be larger: `UNCHANGEDSINCE`, quota `resource-limit`, and `tagged-ext-simple`. So, generic Integer arguments should allow 64 bit numbers. We can add specific argument validation where it makes sense to do so.
This guards against numbers like `99999999999999999999`. This isn't a security issue unless `max_response_size` is `nil`. But it's reasonable to block too large numbers in both the response reader and the response parser, regardless of that config setting. Note also that this still _allows_ strings like `000000000000000001`. This is goofy, but it's how the RFCs are written! ---- 🍒 pick note: `NumValidator` in v0.5 doesn't have `coerce_number64`. Rather than backport _that_, I opted to simply copy a simplified version into both `ResponseReader` and `ResponseParser`.
`#disconnect` could deadlock when called inside the mutex, because the receiver thread could be signaling a condition variable and it would thus be unable to resume until the current thread releases its lock. Also, `#disconnect` shouldn't raise `IO#shutdown` exceptions on the receiver thread prior to closing the socket, when it is being called from the receiver thread. The exception is still raised, _after_ the socket is closed.
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 still allows the argument to be a single string with multiple space-delimited arguments. It splits the string first, and validates the substrings.
Zero-length literals are explicitly allowed by the RFCs and this did not
catch text that ends with `{0}` or `{0+}`. This leaves RawData able to
absorb the `CRLF` that ends the command, and thus absorb the following
command into itself.
Ultimately, we don't care if the `number64` is encoded correctly nor
whether it claims to be a binary literal. So I've simplified the regexp
by dropping `~?` and using `\d+` for the number.
It's bad behavior to send a non-synchronizing literal that's too large. This forces the server to choose between reading but ignoring the bytes or closing the connection. But, for servers that _don't_ support non-synchronizing literals, this could be another CRLF/command injection attack vector. If a server sees the `}\r\n` but can't parse the literal bytesize, it may decide to close the connection, and all is fine. But, a server _might_ respond to any unparseable command line (ending in `CRLF`) with `BAD`, then interpret the literal as the next command. In that case, a CRLF/command injection could succeed. Fortunately, `LITERAL-` is supported by most IMAP servers. So this is not expected to be widely exploitable.
hsbt
changed the title
The non-synchronizing literal tests from the #701 backport used endless method definitions, which require Ruby 3.0. The v0.4 line still supports Ruby 2.7.3, so this broke CI on 2.7.
hsbt
force-pushed
the
backport/v0.4/security-patches
branch
from
e90f822 to
36489fb
Compare
When a client closes the socket abruptly, the server thread's `run` ensure and the test helper's `shutdown` can call `close` concurrently. Without a mutex the `state.logout?` check races, so both callers run `state.logout` and the second raises "already logged out", flaking the send_literal tests on ubuntu CI.
hsbt
force-pushed
the
backport/v0.4/security-patches
branch
from
36489fb to
8a238ad
Compare
|
@hsbt Sorry for the slow reply. I've been on family vacation, without any cell signal! I was using a "critical security fixes only" approach to v0.4.x ever since the last release, so I had intentionally decided to not backport those (if only I'd caught them a few weeks earlier). The CVSS 4.0 scores should not mark these as critical, and even those scores are somewhat inflated, IMO. My cell signal right now is very temporary (only a few minutes), and I'm going to be without cell signal again for a few days. So I won't have time to review until then. I'll trust your judgement, if you think they should be merged and released. |
|
@nevans 👋 No worries about the delay at all. Let me think it over on my side. For now, please forget about OSS and enjoy the rest of your family vacation. |
@nevans I want to ask you to backport these CVE-2026-47240, CVE-2026-47241, CVE-2026-47242 to net-imap 0.4.x too. Because I think some users running critical system still may want us to fix these CVEs with low CVSS score in Ruby 3.3. |
Sorry I misunderstood this was an issue ticket. But this is PR ticket. So, my ask is if you have CVEs with low CVSS in the future, I want you to consider backporting the fix to net-imap versions (in this case, including 0.4.x) to be included in every supported Rubies too, because some users running critical system may want us to fix these CVEs. |
|
Nowadays, zero-CVE container images are used in some use cases. I think that means some users prioritize unfixed CVE number in some use cases rather than CVSS score of CVE. The following article describes this topic as a reference. Zero CVE Container Images - safeguard.sh |
|
@hsbt I think my original intention was to mirror the ruby release policy for the version of |
|
@hsbt I'd previously skipped backporting several more severe security fixes to v0.3, for the same reason. The related CVEs (and the final net-imap v0.3) were released shortly after ruby 3.2's final release and EOL, but only because I took too long preparing them. Another user asked for the other fixes to be backported to v0.3 and I described my decision to not backport all of them here: #663 (comment). (Sorry, my tone there was a bit grumpy.) I would prefer not simultaneously maintain four versions of net-imap (once 0.7 is released). Since the minimum ruby version is only bumped for |
|
@junaruga I think it's completely reasonable to include a "critical security fixes only" release series, as a way of pushing users towards upgrading. If users want to maintain zero CVEs, they still get two major versions with full security support. Since net-imap releases |
|
But, my original goal was to approximately match ruby's release policy, and I was wrong about what that policy is! So I suppose we'll need to release these backports. I'll take a closer look at merging and releasing this PR tomorrow. |
|
This is also somewhat complicated because most of the CVEs have expanded the scope of But this was not well documented, so some users probably have used it more naively, in use cases that require a different threat model. All of When I first started using Because So, if your software is in that smaller set of use-cases where you are affected by these CVEs, please just upgrade! |
@nevans Thanks for the explanation. Yes, I understand your initial decision backporting critical security fixes only is reasonable. As my information, I am working for Red Hat Enterprise Linux (RHEL) at Red Hat, and I am familiar with such use cases changing actions (fix or not fix) depending on the severity (such as CVSS) of CVEs in RHEL. To be fair, even if you decide not to backport the low CVSS CVE fixes to any stable branches, that is also reasonable (it has a reason). I think most different decisions are reasonable from different perspectives, but a factor to differentiate a decision is which reason is more prioritized than other reasons. I am glad to see that you said that you will work on backporting the low CVSS CVE fixes to net-imap 0.4.x, by following Ruby project's release policy. Thanks for your work. |
3 participants
Ruby 3.3 bundles net-imap from the 0.4.x line, which still has no fixed release for the three advisories published on 2026-06-09 (CVE-2026-47240, CVE-2026-47241, CVE-2026-47242). They are fixed in 0.5.15 and 0.6.4.1, but the latest 0.4 release is 0.4.24.
As the Ruby 3.3 maintainer I would like a release that carries the behavior changes required by the security fixes, while staying free of the other 0.5.x incompatibilities. Moving 3.3 from 0.4 to 0.5 in a patch release would pull in
SequenceSet, theMessageSetdeprecation, the config object, and a higherrequired_ruby_version, none of which belong in a maintenance update.So I went through v0.5.14..v0.5.15 and backported the equivalent commits onto
v0.4-stable, mirroring your0.5.15backport in #703. I kept only the security fixes and the prerequisites they depend on, adapted them to the 0.4 code (which still targets Ruby 2.7.3 and usesMessageSetrather thanSequenceSet), and left out everything 0.5-only. The full suite is green, and there are no breaking public API changes beyond the intended rejection of the malicious inputs.How do you feel about cutting 0.4.25 from this branch? If it helps, I am happy to handle the release work myself.