Skip to content

The origin host is matched as a substring, so a third party host is rewritten to the proxy #1119

Description

@jwrosewell

A third-party host whose name merely begins with the publisher's origin host has its front rewritten to the proxy, so the visitor is sent to a host nobody configured.

With origin.example.com as the origin and test.example.com as the request host, this input:

<a href="https://origin.example.com.cdn.example/asset.js">

comes back as:

<a href="https://test.example.com.cdn.example/asset.js">

origin.example.com.cdn.example is a different registrable domain that happens to start with the origin.

Cause

rewrite_url_value in crates/trusted-server-core/src/html_processor.rs (lines 335 to 361 at 0f8b44dc0) builds https://<origin>, http://<origin> and //<origin> as literals and applies them with three str::replace calls at lines 346 to 349. str::replace matches a plain substring, so it has no notion of where a hostname ends.

There is already a boundary check immediately below, at lines 351 to 358, and host_rewrite.rs already has a boundary-aware rewrite_bare_host_at_boundaries. Neither catches this, because the existing check runs after the three replacements, on a string that has already been rewritten. The bare-host path is guarded and the scheme-qualified path is not.

How it was reproduced

By adding a test to your own html_processor suite at 0f8b44dc0, using the existing create_test_config harness, and running the document above through StreamingPipeline. The output above is what the test observed. The test fails on main and passes with the fix.

Suggested fix

Match the origin on whole-host boundaries for the scheme-qualified forms too, rather than building per-scheme literals and replacing them as substrings.

A branch is ready at fix/origin-host-boundary. One commit, two files, based on 0f8b44dc0, merges cleanly against main. It includes the end-to-end test above, so the defect is covered at the level it occurs rather than only in the new helper.

It also brings imagesrcset into line with srcset. The two attributes carry identical markup and srcset had a bare-host pass that imagesrcset did not. These cannot be separated, because fixing the boundary match removes the helper methods the old imagesrcset code called, so a change carrying one without the other does not compile.


Written with AI assistance and checked against the source at 0f8b44dc0. Worth a human review before acting on it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions