Permissions were compared only when they were what a change touched, and
not at all on delete, so an administrator with account-update rights
could reset the password of an administrator holding more, or delete
the account. Changing or deleting an account now needs every permission
it has.
SCIM keeps managing every account on its domains, administrators
included (SCIM-13). Requests through the SCIM endpoint skip the new
check, and the permission change they make is checked for what it adds
rather than for everything the account holds, so SCIM can turn off an
administrator's sign-in and still can't grant anything its key lacks.
A MOVE of a whole collection out of another account checked only that
the caller could remove items from it, then took the collection from its
owner. A JMAP calendar share with mayWriteAll gives that without
mayDelete, so a sharee could move the owner's calendar into their own
account. MOVE now needs Delete as well as RemoveItems on the collection,
as DELETE does. Address books get the same check; there RemoveItems only
ever comes with Delete.
FileNode/set checked a sharee's rights on the node itself but never on
the folder: anyone a folder was shared with could create files in it,
read-only or not, and anyone who could rename a node could move it into
any of the owner's folders or to the top of the account. FileNode/copy
already checked the destination. Creating now needs mayAddChildren on
the folder, and moving needs mayDelete on the folder it leaves and
mayAddChildren on the one it goes to, never the top. A writing delegate
(AL-7) is unaffected.
With onExists=replace, a copy that collided with an existing node put
that node on the list to delete before checking whether the caller
could add to the folder, and nothing took it off again when a later
check refused the copy. The node and everything under it were then
deleted with no Delete check. A sharee who knew a folder id and a name
in it could delete any of the owner's files. The folder is now checked
before names are looked at, replacing needs Delete on the node and all
under it, as FileNode/set's destroy does, and the replaced node is
deleted only when the copy is written.
A property key whose first part holds an escaped "/" (~1) is parsed
again as a pointer, and each level can escape the one inside it. A key
built that way and nested a few thousand deep, well inside the default
request size, recursed once per level: on a 2 MiB stack, like a tokio
worker's, it overflowed and aborted the server. Past 8 levels the text
is now kept as a plain key, which is also what RFC 6901 says "~1" means.
Each recipient's address and local part were searched for in the whole
Subject, and nothing limits how long a Subject or a To header can be.
Both long took time proportional to the two lengths together: 7.4 s for
5,000 recipients and a 2 MB Subject, about a minute for a 1 MB To and a
1 MB Subject, per inbound message. Only the first 1,000 characters of
the Subject are searched now.
The similar-recipients check ran an edit distance over every pair of
To/Cc addresses. A To header with thousands of addresses took time
quadratic in its length: 15 s for 5,000, hours for a large header in
one inbound message. The check now looks at the first 50, which is
plenty to tell a list of near-identical addresses from a real one.
The tokenizer that finds URLs and email addresses in a message looked
for the closing bracket of an IPv6 host with no limit, buffering every
token after an unclosed "[", and then took those tokens off the front
of a Vec one at a time. A long "a@[1:1:1:..." run in inbound mail took
time quadratic in its length: 7.4 s for 200,000 characters, hours for
a full-size message. No IPv6 literal is longer than 47 characters with
its brackets, so the search stops there, and the buffer is a VecDeque.
TLS-RPT report addresses come from DNS, and quoted-printable in a
rua=mailto: URI can put CR/LF into one. The address went into RCPT TO
as it was, so the line break started a command of the record owner's
choosing. Addresses with control characters, spaces or angle brackets
are now refused when they're queued, and any already in the queue fail
instead of being sent.
The client address came from the left-most X-Forwarded-For or Forwarded
entry, which the client writes itself, so a banned or rate-limited client
behind a proxy could pick any address. Headers are now read only when the
connection comes from a trusted proxy, and the client is the right-most
hop no trusted proxy accounts for.
Trusted by default: loopback and the private ranges. INBUXA_HTTP_TRUSTED_PROXIES
replaces that list. A direct client's headers are ignored, and scanner bans
apply to it even with forwarding on.
remove() XORed the bit, so removing an item that wasn't set added it.
IMAP SETACL with "-<rights>" removes rights one at a time through it, so
"SETACL mailbox user -x" against a user without "x" granted them "x".
The JMAP callers remove from a full set, where both forms agree.
Test: removing an unset item left it set on main; it now stays unset.
The topological sort of creation references (#id references between
creates in one /set or Blob/upload) never remembered which creates it had
already placed. When creates share references, e.g. each one referring to
the next two, it walked the same nodes again and again, about fib(n)
times: a request with 60-80 such creates kept a worker busy indefinitely.
The 1000-deep stack check only catches cycles, and this graph has none.
A create that has already been placed is now skipped. Its references were
placed before it, so the order is unchanged.
Test: sorting a 60-create chain did not finish within 2 seconds on main;
it now returns at once, with every create after the ones it references.
STORE ... (UNCHANGEDSINCE n) worked out which requested messages no longer
exist by expanding every number in the client's sequence set into a hash
set, so a single "UID STORE 1:4294967295 (UNCHANGEDSINCE 1)" from any
logged-in user asked the server for billions of entries while it held the
mailbox state lock.
The missing set is now computed as ranges. Numbers up to the mailbox's
highest UID (or message count) are checked one by one; everything above
it is missing by definition and is emitted as a single range without
being walked. MODIFIED carries ranges and is serialized as a sequence set,
so its output is unchanged. Sequence::expand had no other caller and is
removed.
Tests: a range test that failed on main (expand(10) of 1:5000000 returned
5,000,000 entries) now passes against the new code, plus a reference
comparison of the range result against per-number expansion.
jcoffey-dev