Commit Graph

324 Commits

Author SHA1 Message Date
Oswald Buddenhagen
6b22c837f6 overflow-check ranges[] in imap_set_range()
amends 3d64f167.
2022-06-19 16:00:09 +02:00
Oswald Buddenhagen
87c2ac1cc9 reserve enough UID ranges in imap_load_box()
in certain configurations, under very unlikely conditions (which are
practically impossible to control remotely), we'd overflow ranges[].
in a typical gcc build, the values (which are also practically
impossible to control remotely) would be written at the end of buf[],
which would be rather harmless, as only a tiny part of buf is used
subsequently. so i'm not classifying this as a security issue.

amends 77acc268.
2022-06-19 16:00:09 +02:00
Oswald Buddenhagen
127003ee37 reject unreasonably long mailbox names from IMAP LIST
this wasn't really a security problem, as the name mapping we actually
do does not change the string length, and the iteration was already
safe after the literal length fix, but it's still better to catch weird
input.
2021-12-01 10:07:40 +01:00
Oswald Buddenhagen
463272eab8 CVE-2021-3657: reject excessively large IMAP literals
we didn't limit the 32-bit size of literals so far, which, given that we
use int-sized lengths & offsets, permitted all kinds of buffer
overflows. malicious/compromised servers may have been able to exploit
this. actual email senders would be constrained by size limits for
delivered mails, and to cause more than a crash they'd have to predict
the exact size of the final message.

we now limit to 2GB, which, given that we use unsigned ints since
e2d3b4d55 (v1.4.0), gives the handlers downstream plenty of headroom.

an alternative would have been using 64-bit offsets, but this seems like
major overkill, even if IMAP4rev2 recently mandated it (we talk only
IMAP4rev1, so we can ignore it).
2021-12-01 10:07:24 +01:00
Oswald Buddenhagen
a846ab054d enable embedding arbitrarily long strings into IMAP commands
the AUTHENTICATE command may get insanely long for GSSAPI when SASL-IR
is available. instead of growing the buffers each time someone hits the
limit (as done in f7cec306), remove the limitation altogether.

imap_vprintf() still contains a fixed-size buffer which could overflow
when really long strings (e.g., mailbox names) need to be quoted. this
seems very unlikely, so we'll deal with it if someone actually hits it.

REFMAIL: 87sg1qxdye.fsf@cern.ch
2021-06-11 18:24:00 +02:00
Oswald Buddenhagen
444601a1e0 Merge branch '1.3' into 1.4
Conflicts:
	configure.ac
	src/drv_imap.c
2021-06-03 11:04:56 +02:00
Oswald Buddenhagen
589d2ed428 CVE-2021-3578: fix handling of unexpected APPENDUID response code
if the code was sent in response to anything but a STORE, we'd overwrite
a data pointer in one of our imap_cmd subclasses, an allocator data
structure, or the start of the next allocation, with an int that was
completely under the server's control. it's plausible that this could be
exploited for remote code execution.

to avoid this, we could ensure that the object is of the right type
prior to casting, by using a new flag in the parameter block. but it's
easier to just dispose of the out_uid field altogether and reuse the uid
field that is present in the parameter block anyway, but was used only
for FETCH commands so far.

this problem was found by Lukas Braun <koomi@moshbit.net> using a
fuzzer.
2021-06-03 11:02:23 +02:00
Oswald Buddenhagen
a86e6f8c7c don't crash on malformed CAPABILITY responses
amends 95a83c822.

this problem was found by Lukas Braun <koomi@moshbit.net> using a
fuzzer.
2021-06-02 15:51:23 +02:00
Oswald Buddenhagen
d8feb67dae tolerate INBOX mis-casing in Path
while it's technically reasonable to expect the user to match the
server's casing of INBOX if they set Path, this might come as a
surprise to those who know that the IMAP INBOX is case-insensitive.
so tolerate any casing instead. as a minor side effect, we'd now even be
able to deal with a server using different casing in NAMESPACE and LIST.
2021-03-19 18:21:34 +01:00
Oswald Buddenhagen
4b185e35fe Merge branch '1.3' into 1.4
Conflicts:
	configure.ac
	src/drv_imap.c
2021-02-21 21:26:54 +01:00
Oswald Buddenhagen
fe5d59f8e3 CVE-2021-20247: reject funny mailbox names from IMAP LIST/LSUB
in particular, '..' in the name could be used to escape the Path/Inbox
of a Maildir Store, which could be exploited for stealing or deleting
data, or staging a (mild) DoS attack.
2021-02-21 20:40:22 +01:00
Oswald Buddenhagen
95a83c8220 be more tolerant of formally malformed response codes
fastmail sends flags containing ']' in PERMANENTFLAGS, which is formally
illegal. however, if we parse the embedded list before looking for the
response code's closing ']', things work out fine.

as a side effect we won't complain about similarly or completely
malformed response codes we don't recognize at all, which may or may not
be considered an improvement ...
2021-02-14 23:47:14 +01:00
Oswald Buddenhagen
8c86f34bf0 fix bogus continuation of IMAP list parsing
on error, parse_imap_list() needs to reset the nesting level in the
state, as imap_socket_read() uses that as an indicator whether list
parsing is ongoing.
2021-02-14 23:47:14 +01:00
Oswald Buddenhagen
32392adbe3 accept unsolicited FETCH responses (without payload) after all
while the spec says that the server SHOULD not send FETCH responses
about STORE FLAGS when .SILENT is used, at least gmail and fastmail seem
to do it nonetheless. also, in case of concurrent flag updates on the
affected messages such responses can be legitimately sent.

in earlier versions of mbsync this would lead to duplicate messages
piling up in the store, though that would pose no problem at that point.
2021-02-14 23:47:14 +01:00
Oswald Buddenhagen
062706fcbf Merge branch '1.3'
Conflicts:
	configure.ac
	src/drv_imap.c
2021-02-03 15:53:05 +01:00
Oswald Buddenhagen
c8b73acad2 unbreak handling of 'INBOX.' NAMESPACE again
INBOX matching must not prevent prefix (namespace) stripping, as INBOX
may be the namespace.

amends 04fc586e7.

REFMAIL: 186391612191752@vla1-ea7e194e8506.qloud-c.yandex.net
2021-02-03 14:43:11 +01:00
Anton Khirnov
fc300fd811 Set authentication id for the SASL EXTERNAL mechanism
The SASL library will refuse to use the EXTERNAL module when no auth id
is set a priori.

Tested to work with Dovecot, using TLS client certificates for
authentication.
2021-01-05 19:50:21 +01:00
Oswald Buddenhagen
e67cf01eb8 improve SASL error messages
provide context, and remove the redundant numeric codes.
2021-01-05 19:46:29 +01:00
Oswald Buddenhagen
c2e6e962b5 tune SASL-related comments
- add explanations to the callbacks
- remove bogus comment - EXTERNAL can be in fact missing (when no
  authentication id is set)
2021-01-05 19:46:29 +01:00
Oswald Buddenhagen
4423a932f3 add forced async mode to proxy driver
to test async operation of the syncing core while using the synchronous
maildir driver, we add a mode to the proxy driver where it queues
callback invocations to the next main loop iteration.
2020-12-19 13:22:29 +01:00
Oswald Buddenhagen
c3d91ae1e8 introduce new inheritance model based on C11 anonymous structs
the struct declarations got uglier, but their usage requires a lot fewer
explicit references to the parent struct (though some are added where
using the derived struct is more practical now).

we also use something i'd term "covariant members": derivatives of
store_t also reference derivatives of store_conf_t, etc., which
drastically cuts down the number of casts.
fwiw, to achieve this with "proper" inheritance in C++, we'd use
covariant getter functions which hide the still existing casts.

C11 is almost a decade old now, and compilers supported that feature
even longer than that, so i don't expect this to be a problem.
2020-12-17 22:18:10 +01:00
Oswald Buddenhagen
a9ce7be962 streamline init of type & name in imap_parse_store() 2020-10-05 13:15:28 +02:00
Oswald Buddenhagen
09341c10c5 make complaints about unrecognized keywords more verbose
tell the user in what section the keyword appeared, as that may help
spotting mistakes like stray empty lines.
2020-10-05 13:14:48 +02:00
Oswald Buddenhagen
23513564df improve error handling in post-STORE UIDNEXT fallback
that's mostly hypothetical, but let's not make assumptions.

this also adds EXPUNGE response handling to make total_msgs reliable. in
principle, this affects the post-SELECT UIDNEXT fallback as well, but
there the racing window is so short that this barely improves anything.

amends 94022a67.
2020-08-24 12:51:47 +02:00
Oswald Buddenhagen
42f165ecf7 fix UIDNEXT query vs. concurrent imap_fetch_msg()
the uidnext query following message stores can be interleaved with
message fetches. that means that we cannot rely on the 1st command in
flight being that query. but instead of iterating over all commands in
flight, move the uidnext query flag to imap_store (and make sure to
check for the presence of a message body before testing it) - this
avoids the loop and an extra byte in every command.

this also makes it clear that the query is mutually exclusive with
loading messages (the untagged responses are not distinguishable).
2020-08-24 12:51:47 +02:00
Oswald Buddenhagen
f099141e42 make item tracking in parse_fetch_rsp() more uniform
amends 67ea5bea7 & a5a8783ea.
2020-08-24 12:51:42 +02:00
Oswald Buddenhagen
ec47c90554 delay allocation of msgdata.msgid field
this allows us to simplify the exit path of parse_fetch_rsp().
2020-08-05 17:59:28 +02:00
Oswald Buddenhagen
b37d6b1c00 fix invalid free() in error path
the tuid isn't actually allocated - it's a pointer into the raw data.

amends a5a8783e.
2020-08-05 17:36:35 +02:00
Oswald Buddenhagen
c69718baab remove redundant zero initializations
we already use calloc().

amends 130664b6.
2020-08-05 17:29:58 +02:00
Oswald Buddenhagen
b148fd9e44 de-duplicate exit paths of imap_alloc_store() 2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
c83330ffe8 don't unnecessarily re-initialize some members of imap_store
... when recycling server connections.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
cfaa4848dd actually implement imap_commit_cmds()
delay reporting success of STORE FLAGS until a subsequent CHECK
succeeds.

this fixes (inverse flag change propagation) and (deletes not being
propagated) after an interruption due to prematurely logged flag
updates.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
70bad66129 create placeholders for messages over MaxSize
this is vastly more useful than just omitting the messages with no
indication at all.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
0e5046e14a add/fix/de-duplicate comments 2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
395f802500 fix loading of some messages' sizes in some partial sync scenarios
we need to pass a different "boundary" UID to driver_t::load_box() for
every OPEN_* flag that queries a partial range:
- OPEN_FIND refers to messages newer than all we know about
- OPEN_OLD_IDS refers to messages which are paired
- OPEN_{OLD,NEW}_SIZE refers to messages (not) above the committed
  boundary of already propagated messages

we treated the 3rd like the 2nd, which was just wrong - the actual
boundary may be lower or higher, so we'd produce wrong results when
MaxSize was set and only one of New and ReNew was requested.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
198ca65b6e add option to get password from macOS Keychain
this is better than using PassCmd, as it allows the keychain manager to
identify the calling process and therefore use a selective whitelist.

unlike in the now removed example, we use an "internet password" for the
imap protocol, rather than a "generic password" - this seems more
appropriate.

based on a patch by Oliver Runge <oliver.runge@gmail.com>
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
03b15dbdd3 add ability to script IMAP user query
It was already possible to retrieve passwords from arbitrary commands.
But this goes only half the way to allowing automated derivation of
login credentials, as some environments may also have different user
names based on the system. Therefore, add the UserCmd option to
complement PassCmd.

Based on a patch series by Patrick Steinhardt <ps@pks.im>
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
503478533c de-duplicate FETCH response data item traversal somewhat 2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
8acf56b311 complain about malformed item names in FETCH responses 2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
47b477b3fb re-nest parse_fetch_rsp()
prefer early exits over else branches, which is easier to follow.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
81c4bfeefa extract parse_fetched_flags() from parse_fetch_rsp() 2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
d4ead05a02 extract parse_fetched_header() from parse_fetch_rsp() 2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
67ea5bea7f handle bogus IMAP FETCH responses more robustly
don't use assert()s when the error condition can stem not only from
errors in mbsync's logic, but also from the IMAP stream being corrupted.

amends 72be55b0e.

REFMAIL: 20191021233411.55ctuvslkfqf2pna@koblih.localdomain
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
a5a8783ea3 sanitize error handling in IMAP FETCH response processing
abort on actual error conditions (protocol errors) and downgrade the
rest to warnings.

REFMAIL: 20191102164509.dxayakg3hrmozjnm@carbon
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
b91dd5b3bc centralize disposal of parsed IMAP lists
makes the code less cluttered, and it's harder to introduce leaks.

this has the hypothetical disadvantage that due to freeing being
delayed, the peak memory usage would rise significantly if we chained to
another parse_list() call which produces a big list while already
holding a big list, but that isn't the case anywhere.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
7af7354dbc fully decompose NAMESPACE response early on
that way the code becomes clearer, and we don't keep useless nodes in
memory.
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
6fd4e8de24 don't store 'shared' and 'other' namespaces
they are never used anyway, and aren't going to be (because configuring
that would be more annoying than just specifying Path manually).
2020-08-04 17:16:03 +02:00
Oswald Buddenhagen
64e5f07ad3 consistently use NULL for null pointers
makes the code noisier, but also somewhat more expressive.
2020-08-04 17:16:01 +02:00
Oswald Buddenhagen
e2d3b4d55b fix lots of sign conversion warnings
... by making a lot of objects unsigned, and some signed.
casts which lose precision and change the sign in one go (ssize_t and
time_t to uint on LP64) are made explicit as well.
2020-08-04 17:15:39 +02:00
Oswald Buddenhagen
cc176df2c3 make some narrowing of integers explicit
this does specifically *not* cover about a bazillion warnings about
size_t being shrunk to uint - these make no sense given the expected
data set size.
2020-08-04 17:14:55 +02:00