DoSMimeDecryptMessage and DoSMimeVerifyMessage built BODY.PEEK[<partId>]
(and [<partId>.MIME]) from the partId / sigPartId parameters without checking
them, so a value could close the bracket or the line and add IMAP commands of
its own. They run in the user's own IMAP session, so this is not cross-account,
but nothing in that session should be steerable by a request parameter.
Read both through smimePartIdParam(), which accepts only an IMAP section number
(1, 1.2.10) or TEXT and refuses anything else.
test/smime-partid.php checks valid ids pass and injection attempts, zero or
leading-zero sections, lowercase and other section keywords are refused.
With 2-step verification enabled, these plugin actions needed nothing but a
session (and the CSRF token that comes with it):
- EnableTwoFactor with Enable=0 switched it off,
- ClearTwoFactorInfo deleted it,
- CreateTwoFactorSecret overwrote the secret with Enable false, i.e. also
switched it off,
- ShowTwoFactorSecret returned the secret and QR code.
A stolen cookie or an unlocked machine was enough to drop or copy the second
factor. While 2FA is enabled, all four now require a current TOTP or backup
code, checked as at login; a backup code used here is spent. Enabling and the
first setup are unchanged. Backup codes come from random_int() instead of rand().
The settings page asks for the code (the existing "Verification Code" label)
before those actions and sends it. A refused change reloads the real state;
before, any error made the page show 2FA as not configured. A cancelled or
refused "off" puts the checkbox back. TwoFactorAuthSettings.min.js, which is
what is served, is now the same file as the source rather than a separate
hand-minified copy.
test/two-factor-reauth.php drives the real gate with storage and provider
stubbed: open while not enabled; refused with no, a wrong or an unknown backup
code while enabled; allowed with a valid TOTP or backup code, which is spent.
URIHasPublicHost() relied on FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE
alone. On PHP 8.4 that passes 100.64.0.0/10 (shared space: CGNAT, Tailscale,
Alibaba metadata at 100.100.100.200), 192.0.0.0/24, 198.18.0.0/15,
224.0.0.0/4, fec0::/10 and ff00::/8. It also passed the server's own public
addresses: a fetch to them never leaves the host, so it reaches services that
listen on every address but are firewalled from outside (on our host: grafana,
forgejo, prometheus and the exporters).
Both checks, on the literal host and on every resolved address, now go through
IsPublicIP(): PHP's flags, then an explicit list of the ranges above, then the
addresses net_get_interfaces() reports for this host (cached per request).
test/ssrf-ranges.php checks the new ranges and their edges, that ordinary
public addresses still pass, and the own-address rule via the cache. Before
this change 100.64.0.1 was treated as public. DNS rebinding between check and
connect is still not covered; that needs the connection pinned to the checked
address.
Raw view served an attachment inline with the Content-Type from the message,
only forcing PDF by file name. A part named photo.png declared as text/html is
previewed as an image by extension, and its link opened an HTML page on the
webmail origin. The raw CSP (script-src 'none'; child-src 'none') stopped
scripts, but replaced the global policy, so forms, meta refresh, base and
remote styles and images all worked: a convincing phishing page under the real
hostname.
- inlineContentType() allows only raster images, PDF, audio, video and
text/plain to be shown as declared. Other text/* is shown as text/plain (an
HTML attachment displays its source); anything else, SVG and XML included,
is sent as an application/octet-stream download.
- The raw CSP adds default-src 'none', form-action 'none' and base-uri 'none',
with explicit img, media, style and object sources for what raw view
legitimately shows. object-src 'self' keeps the browser PDF viewer usable
(not verified in a browser here).
test/raw-inline-type.php covers the type decision, parameters and case.
ServiceTest() ran Integrity::test() for any request. The test builds its base
URL from $_SERVER['HTTP_HOST'], sends HEAD requests to it and prints the status
and every response header. The client therefore chose where the server sent
requests and read the result. Behind nginx the vhost is picked by server_name
with the port ignored, so Host: <this host>:<port> reaches the app and turns the
diagnostic into a probe of the server's own TLS ports and their headers.
Answer 404 unless an admin session is present. An admin's browser sends the
real Host, so the diagnostic still works for the people it is for.
Checked end to end with php -S (4 workers, no network): without a session the
old code returned 200 with the diagnostics, this one returns 404 and nothing.
Crypt::Decrypt() calls "{name}Decrypt" with the algorithm name taken from the
token itself, so a client could ask for xxtea on any server. XxteaDecrypt used
$salt . Passphrase() as the key, but XXTEA reads only the first 16 bytes of its
key and the salt is 16 bytes stored in the token: the server secret never took
part. Anyone could therefore make a token every Crypt::Decrypt* consumer would
accept as genuine (remember-me, secure cookies, DecodeKeyValuesQ raw keys, ...),
and any real xxtea token could be read. The consumers traced so far still
require a server-side record or a real IMAP login, so this is not a known
account takeover, but the integrity check they rely on was not there.
- Decrypt refuses xxtea whenever sodium or openssl is available, i.e. whenever
Encrypt() would never have produced it, compares the name case-insensitively
(PHP method names are), and dispatches only sodium, openssl and xxtea rather
than any method name ending in Decrypt.
- The xxtea key is now the first 16 bytes of sha256(salt . passphrase), so on a
host left with nothing but xxtea it depends on the secret. Tokens written under
the old derivation stop decrypting there; they were never keyed by the server.
test/crypt-forgery.php forges a token with an attacker-chosen salt (accepted
before this change), checks it and an arbitrary method name are refused, and
that sodium, url-safe and xxtea round trips still work only with the right key.
The sanitizer sends img src, and url() in background-image, list-style-image
and content, through the remote content handling: blocked until the user
allows it, rewritten to the proxy when it is on. Two other ways to load an
image were left alone:
- srcset (and sizes) were on the attribute allowlist and never rewritten, so
<img srcset="https://..."> loaded when the message opened. With the proxy on,
the CSP keeps img-src to 'self', but a relative srcset="?/ProxyExternal/<b64>"
is same-origin and made the server fetch an attacker URL at render time: a
read receipt with no click. With the proxy off the browser fetched directly.
- url() in any other style property (border-image-source, mask-image, filter,
...) kept its value, with the same two outcomes.
Drop srcset and sizes from the allowlist (attributes not on it are deleted),
and remove any other style property whose value holds url(). Relative img src
was already safe: anything not cid:, http(s) or data:image/ is parked as
data-x-src-broken.
Checked the style sweep in Chromium: border-image-source, mask-image (both
spellings), filter and cursor lose the url(), other declarations survive, and
the background/list-style shorthands are left to the existing handling.
OpenSSL::verify() called openssl_pkcs7_verify with PKCS7_NOSIGS, which skips
the signature check, and returned success whenever that call did. Any
well-formed signed message therefore came back as success: true, including one
whose content had been changed after signing, and the client shows that as a
good signature (Message.js smimeSigned). Applies to the message view, the
explicit verify action and signed content inside decrypted mail.
The call is also how the content of an opaque signed message is extracted for
display, so it stays as it is for that, and a second openssl_pkcs7_verify
without PKCS7_NOSIGS now decides success and does not throw on failure. A
message with a broken signature remains readable but is no longer reported as
signed. PKCS7_NOVERIFY is kept on purpose: the signer certificate is not checked
against a CA, so success means the content is what the certificate holder
signed, not that the certificate is trusted; the comment says so.
test/smime-verify.php signs detached and opaque messages with a throwaway key,
checks the intact ones verify, and that altering one byte of signed content
flips success to false while the content stays readable. Before this change
the altered detached message was reported as validly signed.
DecodeHeaderValue removed CR and LF before decoding encoded-words, so an
encoded-word could still decode to a line break. A message whose Message-ID
was =?utf-8?b?...?= with CRLF inside reached the client with the break intact,
the client copies Message-ID into In-Reply-To when replying or forwarding, and
SetInReplyTo only trimmed it. The reply then carried header lines chosen by the
sender of the original, or a body if the payload held a blank line, sent and
DKIM-signed as the user and saved to Sent. Subject on Edit-as-new had the same
shape through the ASCII path.
Strip CR, LF and NUL in two places: after decoding in DecodeHeaderValue, and in
Header itself, which every outgoing header is built through, so a value that
arrives by another route (client parameters, plugins) cannot break the line
either. Folding is unaffected; __toString adds it back.
test/header-injection.php covers B and Q encoded-words, parsing, outgoing
In-Reply-To and Subject, and that long values still fold.
Unfolding appended a newline before the continuation line, which kept its
own leading whitespace; Header turns the newline into a space, so every
fold became two spaces ("et checklist"), carried into reply subjects.
The test now compares exactly.
