Remove dead ad-proxy URL rewriting from Prebid parse_response#531
Open
ChristianPavilonis wants to merge 3 commits intomainfrom
Open
Remove dead ad-proxy URL rewriting from Prebid parse_response#531ChristianPavilonis wants to merge 3 commits intomainfrom
ChristianPavilonis wants to merge 3 commits intomainfrom
Conversation
ChristianPavilonis
changed the title
aram356
requested changes
Mar 20, 2026
Collaborator
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
@ChristianPavilonis Please resolve conflict
Store RequestInfo from the original client request on the provider during request_bids and use it in parse_response for URL rewriting. Previously, host and scheme were read back from the upstream Prebid Server response body, allowing a compromised or misconfigured bidder to inject arbitrary values into ad markup URL rewrites. The request_host and request_scheme fields are still sent to Prebid Server in the TrustedServerExt for the signing protocol, but the response-side values are no longer trusted for rewriting. Closes #417
The transform_prebid_response, rewrite_ad_markup, and make_first_party_proxy_url functions generated /ad-proxy/ URLs whose route handler was removed in 25084ba (NextJS with Prebid Integration). The downstream creative::rewrite_creative_html already rewrites all creative URLs to /first-party/proxy, making the Prebid-level rewriting both dead and harmful (it produced double-rewritten URLs pointing to a non-existent endpoint). Removing this dead code also eliminates the security issue where request_host and request_scheme were read from the upstream Prebid Server response body (#417) — there is simply no response-side URL rewriting left to trust or distrust. Closes #417
ChristianPavilonis
force-pushed
the
fix/prebid-host-scheme-trust
branch
from
18041e7 to
9bce2ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a security issue (#417) where
request_hostandrequest_schemefor URL rewriting were read from the upstream Prebid Server response JSON, allowing a compromised or misconfigured bidder to inject arbitrary values into ad markup URL rewrites.What we discovered
During review we found that the entire
transform_prebid_responsecode path is dead code:Route handler removed —
transform_prebid_responsegenerates/ad-proxy/URLs (e.g.https://host/ad-proxy/adsrvr/...), but the/ad-proxy/route handler was removed in 25084ba ("NextJS with Prebid Integration"). These URLs fall through to the publisher origin fallback, which can't handle them.Downstream rewriter already handles this —
creative::rewrite_creative_html(called fromformats.rsduringconvert_to_openrtb_response) rewrites all third-party URLs in creative HTML to signed/first-party/proxy?tsurl=...&tstoken=...paths usingSettings. This is the active, working rewriter.Double rewriting — When both rewriters were active,
transform_prebid_responsefirst rewrotehttps://cdn.adsrvr.org/pixel.png→https://host/ad-proxy/adsrvr/pixel.png, thencreative::rewrite_creative_htmlsaw that as an absolute URL and rewrote it again to/first-party/proxy?tsurl=https://host/ad-proxy/adsrvr/pixel.png&tstoken=...— a proxy URL pointing to a non-existent route.nurl/burlrewriting was dead work — those fields are parsed into theBidstruct but never used in the finalconvert_to_openrtb_responseoutput.How it was fixed
Instead of patching the untrusted-data path (the original approach used
OnceLock<RequestInfo>to cache local request info), we removed the dead code entirely:transform_prebid_response,rewrite_ad_markup, andmake_first_party_proxy_url(~80 lines)OnceLock<RequestInfo>field fromPrebidAuctionProvider— it was only needed to feed the deleted rewriterRequestInfocapture inrequest_bidsand thetransform_prebid_responsecall inparse_responseOnceLock,base64::BASE64,serde_json::json)PrebidAuctionProviderreturns to being a stateless config-only struct. Therequest_host/request_schemefields are still sent to Prebid Server inTrustedServerExtfor the signing protocol — that outbound path is unchanged.Verification
cargo test --workspace— all 667 tests passcargo clippy --all-targets --all-features -- -D warnings— cleancargo fmt --all -- --check— clean/first-party/proxyURLs appear in auction responses (not/ad-proxy/)Closes #417