Hardening: re-validate stored host settings on load #13

Closed
opened 2026-10-15 22:38:57 -07:00 by eric · 0 comments
Owner

Type: hardening (only reachable by a fully privileged writer). Found by security audit run 3 (commit 01195c8), observed in a local lab.

What happens

Repository::load -> Settings::fromArray -> Host::fromArray (src/Config/Settings.php:69-79, src/Config/Host.php:40-47) only casts stored values; it does not re-run the sanitizer. The sanitizer is attached via register_setting on admin_init, so wp option update (WP-CLI), direct DB writes or other plugin code bypass it. Effects observed with deliberately malformed stored data:

  • an empty host row makes ContentRewriter match every bare // (JS comments, http:// ) and rewrite it to the current host;
  • an empty primary with an unmatched Host emits http:///path redirects and adds '' to allowed_redirect_hosts;
  • an object stored as a host fatals every request (could not be converted to string).

Suggested fix

In Settings::fromArray / Host::fromArray, run each host through HostHeader::normalize, drop rows that come back empty or are non-scalar, validate lang with Locales::isValid, and enforce a single primary (mirror SettingsSanitizer::enforceSinglePrimary). Defence in depth: refuse to emit URLs when the current host is empty. Regression test: Settings::fromArray(['hosts'=>[['host'=>'','is_primary'=>true],['host'=>'www.example.test']]]) yields one host, www.example.test, flagged primary.

**Type:** hardening (only reachable by a fully privileged writer). Found by security audit run 3 (commit `01195c8`), observed in a local lab. ## What happens `Repository::load` -> `Settings::fromArray` -> `Host::fromArray` (`src/Config/Settings.php:69-79`, `src/Config/Host.php:40-47`) only casts stored values; it does not re-run the sanitizer. The sanitizer is attached via `register_setting` on `admin_init`, so `wp option update` (WP-CLI), direct DB writes or other plugin code bypass it. Effects observed with deliberately malformed stored data: - an empty host row makes `ContentRewriter` match every bare `//` (JS comments, `http:// `) and rewrite it to the current host; - an empty primary with an unmatched Host emits `http:///path` redirects and adds `''` to `allowed_redirect_hosts`; - an object stored as a host fatals every request (`could not be converted to string`). ## Suggested fix In `Settings::fromArray` / `Host::fromArray`, run each host through `HostHeader::normalize`, drop rows that come back empty or are non-scalar, validate `lang` with `Locales::isValid`, and enforce a single primary (mirror `SettingsSanitizer::enforceSinglePrimary`). Defence in depth: refuse to emit URLs when the current host is empty. Regression test: `Settings::fromArray(['hosts'=>[['host'=>'','is_primary'=>true],['host'=>'www.example.test']]])` yields one host, `www.example.test`, flagged primary.
eric closed this issue 2026-10-16 11:52:37 -07:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
eric/multi-domain-redux#13
No description provided.