Skip to content

v8.5.33 — a request body is parsed before it is scrubbed

Choose a tag to compare

@marcing marcing released this 22 Sep 07:52
· 0 commits to main since this release
7da3a70

⚠️ Security: the raw request body was scrubbed as free text, and a body is a document

Sender logs the raw request body so the console can replay the request that failed — a
JSON API call's $_POST is empty, so without it a replay is a bare method and URL. That body
went through the same flat <name> = <value> search as an exception message.

One mistake, six holes, all of them shipped:

shape what happened
{"password": "correct horse battery staple"} not redacted at all — the value class excluded whitespace, so the closing backreference could never reach the quote. It did not redact partially; it did not match
opts[api_key]=live_… walked past — [ and ] were not in the name class, and that is the shape every form posts a nested field in
<password>x</password> no XML body was touched anywhere. The separators were =>, : and =; XML writes the name and the value either side of a >
XML-RPC system.multicall names its credentials not at all — the password is the second positional <string>, so no name-based rule could ever find it
{"query":"… login(password: \"x\") …"} the escaped quote was read as the value, leaking the real one and writing a malformed document into storage
user=marcin in a body the username rule never ran on text, so the same field meant different things depending on which channel carried it

Passwords contain spaces. Settings forms post bracketed names. The two most brute-forced
endpoints on the internet speak form-encoding and XML-RPC. Every one of those was a miss.

What changed

Ovos\Service\Console\Body parses rather than pattern-matches. JSON and form bodies are
decoded, handed to the key walk — Logger::remove(), which never had any of these holes
because it walks keys — their string values run through the text scrubber, and the result is
re-encoded.

Parsing also decodes, which is the half no pattern could reach:
note=password%3A%20hunter2 carries no separator for a text search to see, and a whole JSON
blob stuffed into one form field is one opaque token. Both are now read.

XML-RPC is reduced to its shape: every <string> becomes its length while the method names
stay. system.multicall × 200 × wp.getUsersBlogs says more about what is happening than the
passwords did.

Logger::removeText() keeps the same four passes for the places a body is not involved —
messages, backtraces, header values — where the identical holes also bit.

New: console.request_body

off | structure | full, default structure. There was no switch before: the body was
read on every non-GET report, 16 KB of it, at every log level, and nothing could turn it off.
structure parses, drops the fields named like credentials, masks e-mails and re-encodes at
8 KB; full is the previous behaviour.

An endpoint whose purpose is to receive a password — wp-login.php, xmlrpc.php,
/wp-json/jwt-auth/, /oauth/token and the usual siblings — sends no body whatever the mode
says
. The console applies the same rule again on write, which is what reaches a deployment
running an older copy of this library.

Also

  • Logger::maskEmails() is public and isSecretName() joins it — the question "is this
    name a credential?" asked on its own, which a header allowlist needs. A sender was otherwise
    obliged to keep its own copy of the secret-name list just to ask it.
  • Rollup::routeName() takes an optional trailing $module — the ZF1 CMS sites route on a
    module/controller/action triple, so their copy carried a third segment and with it a second
    copy of the validation regex and the __other fallback. Optional and last, so every existing
    caller is untouched.

Together those two let the CMS writers delegate their whole redaction half — 323 lines whose
redact() was Logger::remove() verbatim, kept equal to this library by hand and policed by
a parity script. What that buys is not the lines: it is that "do the copies still agree?" stops
being a question anyone can answer wrongly.

Verification

The rules are pinned by a shared corpus in the console repo
(project/application/tests/fixtures/scrub-body.json, 17 cases) driven through every sender.
This library: 21 checks green, with its own suites at Logger 21/21 and Sender 27/27.

Upgrading

No API is removed and no call site changes. An application that wants the previous behaviour
sets console.request_body: full; one that would rather send nothing sets off.