Re: [PATCH] imap-send: increase command size limit

[Jeff King <peff@peff.net>]

On Sun, Apr 14, 2024 at 06:47:52PM +0200, René Scharfe wrote:

> While 1KB is plenty for user names, passwords and mailbox names,
> there's no point in limiting our commands like that.  Call xstrvfmt()
> instead of open-coding it and use strbuf to format the command to
> send, as we need its length.  Fail hard if it exceeds INT_MAX, because
> socket_write() can't take more than that.

Hmm. I applaud your attention to detail, but this INT_MAX thing is ugly. ;)
Shouldn't socket_write() just use size_t / ssize_t?

In particular, this made me wonder what we would do for larger items.
Like, say, the actual message to be uploaded. And indeed, we use a
strbuf to read in the messages and pass the whole buffer for each to
socket_write(). So we'd possibly quietly truncate such a message.

Fixing it is a little more complicated than switching to size_t, because
the underlying SSL_write() uses an int. So we'd probably need some
looping, similar to xwrite().

In practice I doubt this is ever an issue. 2GB emails are not likely to
be usable in general. And I kind of doubt that this is a reasonable
vector for attacks, since the inputs to imap-send would generally come
from the user themselves (and certainly truncating the attack message is
probably not that interesting, though I imagine one could convince
write_in_full() to do an out-of-bounds read as a size_t becomes a
negative int which becomes a large size_t again).

So I am happy enough with this (especially given my general opinions of
imap-send in the first place).

-Peff