we optimized the case where the string would be equal even without
upper-casing, but for the much more common case where the strings differ
even after upper-casing, this was just an additional conditional.
we used to pass all initially loaded messages to match_tuids(), which
could be quite some when syncing old messages. as lost TUIDs result in
O(n^2) behavior, this could have a serious performance impact.
this is meant primarily for use with a redirected output, where no
progress counters are shown. but it would be weird not to have it with
tty output as well, so it replaces the counters after completion.
REFMAIL: 87bl2cgt6u.fsf@curie.anarc.at
speak more accurately of redundancy, not conflict. these are fatal
errors at all only because the user likely meant something else than
they typed, so we force them to think again.
this moves the channel iteration & synchronization code from main(),
with all its dependencies.
then it is "re-threaded" to be more directly driven by the driver
callbacks (like sync_boxes() is), rather than being a weird state
machine.
while the code is moved, localize many variables, and use an enum
instead of #defines for the states.
it makes no sense to trash the placeholders, and in fact the common
case is that they are deleted due to being replaced by the full
message.
a separate S_PURGED state needed to be added, as S_PURGE needs to be
reset after setting F_DELETED (so the operation doesn't count as still
pending after journal replay), yet trashing needs an indicator. logging
is now done via a separate command, as piggy-backing it on flag updates
just makes things less legible for no benefit.
this is mostly academical, as trashing being done on the side where
placeholders reside is rather unlikely.
... as otherwise these messages would be just lost.
the assumption is that opposite-side trashing is used only for locally
generated messages whose size we control. it's also more consistent with
same-side trashing, where even oversized messages would be trashed.
the exclusion was broken anyway, as we failed to query the size of old
messages, particularly after 70bad661.
messages for which only a placeholder was propagated should be treated
as not propagated, as otherwise the actual contents will be lost when
only not propagated messages are trashed.
amends 70bad661.
this is a lot more legible, and makes it possible to insert values in
the middle without churn.
i didn't find a way to do this with the pre-processor, so we now have
another code generator.
we now use the $< make variable, which requires gmake on netbsd < 9.0,
and possibly other systems with an ancient make.
this makes config+data file "sets" relocatable, which is useful for
testing.
this is technically a gratuitous backwards incompatible behavior
change, but to the degree that anyone uses relative paths at all, they
almost certainly rely on PWD being set up such that they won't see a
difference.
there are fallbacks to the old locations for compatibility.
the new locations use "isync" instead of "mbsync", which is preparation
for renaming the executable back in v1.6.
- wrap flow-controlled statements that contain blocks into blocks
themselves
- wrap bodies of do-while()s into blocks
- use braces on 'else' symmetrically (this obviously has a cascading
effect, so this patch touches lots of lines)
- attach braces
unavoidably, the rules are sometimes broken around #ifdef-ery.
instead of specifying the mailboxes and sync state verbatim, use a
format which deals only with "subjects" (but no UIDs), and specifies the
whole state for each subject on a single line (exceptions prove the
rule).
the dumpers don't try to re-create the abstraction, as that's deemed
to be an unreasonable effort.
while rewriting most of the test data anyway, move it to the bottom of
the file, which is a more natural location for it.
don't abort the comparison if continuing makes sense, and try to be more
specific about the problems.
we give up if messages are excessive/missing or the subject is wrong,
as that touches upon the rather complex problem of diff optimization.
this avoids the ugly and error-prone repeated reading of the state
after a failure.
cmpbox() had to be made non-destructive on the box state.
readchan() had to be created.
parse the test data into hierarchical structures instead of using it in
its raw form. this is semantically cleaner and allows us to change the
input format more easily.
it doesn't really add any value when resuming after an interruption, as
incomplete propagations are perfectly expected, and are recovered from.
but it does make sense in the non-UIDPLUS path, where not finding just
stored messages indicates a problem.
so move the reporting from match_tuids() to msgs_found_new(), and remove
a redundant warning from the latter.
amends 6577bf3e.
while at it, add/fix some licenses/copyrights/comments:
- it makes no sense to have a GPL exception in scripts
- ted did not contribute to the man page
- tst_timers is not part of the mbsync executable
- explicitly put the build system under GPL and add copyrights
manual:
- explain what "rename on move" really means
- reword "remote" to "opposite" to make it less confusing
(possibly renaming TrashRemoteNew left as an exercise for later)
- mention example mbsyncrc
- consistently capitalize Store/Channel/Group where they refer to the
respective configuration entities
- emphasize that SyncState may need a trailing slash (as we do for Path)
- fix missing mention of global usage/default for some options
example mbsyncrc:
- add big fat note that empty lines matter
- stop demoing deprecated options
- point out that CertificateFile is optional
REFMAIL: 877dd11jb3.fsf@angela.anarc.at
the messages are trashed in mailbox (and thus UID) order, and in
practice we expect the operations to complete in order. however, if
older messages need to be trashed after a journal replay, and we get
interrupted again, the next replay would produce an unsorted array,
and thus break the binary search.
amends 2bba9b9.
there isn't really a reason for that; DEBUG_CRASH is quite unlike the
other DEBUG_ flags.
note that the DEBUG_*_ALL flags are not checked, because they always
come with their corresponding less verbose flag anyway.
copy-pasto; it certainly wasn't meant to respond to --debug-sync.
the problem was barely noticeable, as the maildir driver's only debugs
are in the rarely triggered rescan path, apart from the flags usually
being used en bloc anyway.
amends 0e1f8f9a.
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.
the input isn't necessarily null-terminated (it currently is for imap,
but not for maildir), so if the message ended somewhere within the
header field name, we'd read beyond its end, which theoretically could
cause a crash. no other adverse effects could result, as we'd stop
processing such a broken message right afterwards.
amends 70bad661.
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.
that shouldn't really be a problem, as we have 2GB of headroom, and most
growth would happen when sending an all-newlines message from maildir to
imap (due to CR additions), which is mostly non-critical. but better
safe than sorry.
don't try to read messages > 2G, as that will only lead to trouble down
the line.
this wouldn't have worked on linux anyway (we read in one chunk, and
that is limited to (2^31 - 2^12) on all architectures), but on
platforms were big reads work, this was a security problem if one
synchronized other users' maildirs.
as a minor fix on the side, we now also clip the reported message size,
so MaxSize works for excessively big messages.
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).
when a broken/compromised/malicious server gives us a message that
starts with an empty line, we'd enter the path for inserting a pristine
placeholder subject, for which we unfortunately didn't actually allocate
space (unless MaxSize is in use and the message exceeds it).
note that this cannot be triggered by merely receiving a crafted mail
with no headers (yes, it's actually possible to send such a thing), as
the delivery of mails adds plenty of headers.
amends 70bad661.
this is a cheap way to catch symlink loops. 10 seems like a reasonable
limit, as it's unlikely that anyone would be able to actually work with
such a deeply nested mailbox tree.
fixes debian bug #990117.
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
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.
