Skip to content

Protect global ORTB consent fields at the level where the SDK writes them - #1009

Open
mdanylov-sigma wants to merge 1 commit into
masterfrom
fix/global-ortb-protected-fields-levels
Open

mdanylov-sigma wants to merge 1 commit into
masterfrom
fix/global-ortb-protected-fields-levels

Conversation

@mdanylov-sigma

Copy link
Copy Markdown
Collaborator

Summary

OpenRtbMerger.removeSensitiveData strips SDK-computed fields out of the publisher-supplied global ORTB config before merging, so that a publisher can enrich the bid request but cannot overwrite signals the SDK is responsible for. Three of its field lists were applied at the wrong nesting level, so they protected nothing.

The result is a privacy-compliance bug: a global ORTB config could silently override the GDPR and US Privacy values the SDK computed from the consent storage, and could restore location precision the SDK had deliberately coarsened.

This PR re-applies each list at the level where the SDK actually writes the field, matching ArbitraryGlobalORTBHelper.ProtectedFields in prebid-mobile-ios.

Why this needs fixing

1. regs.ext.gdpr and regs.ext.us_privacy were never protected

FIELDS_REGS = {"gdpr", "us_privacy", "coppa"} was applied to openRtbJson.optJSONObject("regs") — the top level of regs. But UserConsentParameterBuilder writes two of those three into regs.ext:

bidRequest.getRegs().getExt().put(GDPR, gdprValue);          // UserConsentParameterBuilder:51
bidRequest.getRegs().getExt().put(US_PRIVACY, usPrivacyString); // :64
bidRequest.getRegs().coppa = subjectToCoppa ? 1 : 0;         // :71 — the only top-level one

So only coppa was ever removed. A publisher config of:

{ "regs": { "ext": { "gdpr": 0 } } }

passed straight through the merger and overwrote the SDK's computed gdpr flag on the outgoing bid request. The same applies to us_privacy.

This is the whole point of the sanitization step: these values come from the CMP via UserConsentManager, and the SDK must be the single source of truth for them. A publisher able to flip gdpr to 0 (whether by mistake in a hand-written config, or by copying an example blob) turns off downstream GDPR handling for every request in the app while the CMP says otherwise.

2. FIELDS_GEO was dead code, and user.geo was unprotected

FIELDS_GEO was applied to openRtbJson.optJSONObject("geo") — a root-level geo object. There is no root-level geo in OpenRTB; geo lives at device.geo and user.geo. The list never matched anything.

device.geo happened to be covered anyway, because FIELDS_DEVICE already contains "geo". user.geo was not covered by anything — and the SDK writes it:

final Pair<Float, Float> userLatLng = TargetingParams.getUserLatLng();
if (userLatLng != null) {
    final Geo userGeo = user.getGeo();
    Integer precision = TargetingParams.getLocationDecimalPrecision();
    userGeo.lat = Util.applyLocationPrecision(userLatLng.first, precision);
    userGeo.lon = Util.applyLocationPrecision(userLatLng.second, precision);
}

BasicParameterBuilder.appendUserTargetingParameters

That applyLocationPrecision call is a deliberate privacy control — the publisher asked for coordinates to be truncated. Leaving user.geo open meant a global ORTB config could put full-precision coordinates back into the request, defeating the setting the app itself configured.

3. regs.gpp and regs.gpp_sid were missing

Both are serialized by Regs.getJsonObject and both are SDK-computed — UserConsentParameterBuilder.appendGppParameter feeds them from UserConsentManager.getRealGppString() / getRealGppSid(). They belong in the protected set for exactly the same reason as gdpr / us_privacy, and iOS already protects them.

What changed

removeSensitiveData now pairs each object with its own ext list, mirroring the iOS props / extProps split:

List Applied at Contents
FIELDS_REGS regs coppa, gpp, gpp_sid
FIELDS_REGS_EXT regs.ext gdpr, us_privacy
FIELDS_USER user geo
FIELDS_USER_EXT user.ext consent
FIELDS_DEVICE device unchanged

FIELDS_GEO is deleted. Geo is now protected wholesale by its parents (device.geo via FIELDS_DEVICE, user.geo via FIELDS_USER), which is what iOS does — no per-field geo list is needed.

Alignment with iOS

ArbitraryGlobalORTBHelper.ProtectedFields in prebid-mobile-ios gets the nesting right today, and its removeProtectedFields(from:props:extProps:) takes both an object-level and an ext-level list per object. After this PR the two SDKs agree on:

  • regsPropsFIELDS_REGS (gpp_sid, gpp, coppa)
  • regsExtPropsFIELDS_REGS_EXT (gdpr, us_privacy)
  • userPropsFIELDS_USER (geo)
  • userExtPropsFIELDS_USER_EXT (consent)

Beyond correctness, parity matters here because publishers ship the same ORTB config JSON to both platforms. Today, a config containing regs.ext.gdpr is stripped on iOS and honoured on Android — the same app sends different consent signals depending on the platform, which is the worst possible failure mode for a compliance signal, since it is invisible unless you diff the two bid requests.

Deliberate divergences (both intentional, both noted in code)

  • regs.ext.tfua stays unprotected. iOS doesn't protect it either, and per [Android SDK] Inquiries on Regional Child Privacy (tfua) & Impression-Level ORTB Config #997 the global ORTB config is currently the only way for publishers to send tfua for non-US child-protection regimes. There is now a comment on FIELDS_REGS_EXT so this isn't "tidied up" by a future change. A regression test covers it.
  • device.ext is still dropped wholesale on Android. FIELDS_DEVICE contains "ext", so the entire device.ext object is removed; iOS only strips atts and ifv from it. Android's behaviour is the stricter of the two, and narrowing it would loosen protection, so it is left alone here. Worth a follow-up decision, but out of scope for a fix.

Tests

Five new tests in OpenRtbMergerTest:

  • mergeSensitiveData_regsExtConsentFields_areNotOverridden — publisher regs.ext.gdpr / us_privacy cannot overwrite SDK values
  • mergeSensitiveData_regsExtConsentFields_areNotAddedToEmptyRequest — nor can they be injected when the SDK set nothing
  • mergeSensitiveData_regsExtTfua_passesThroughtfua survives while a sibling gdpr in the same object is stripped
  • mergeSensitiveData_regsCoppaGppAndGppSid_areNotOverridden
  • mergeSensitiveData_userGeo_isNotOverriddenuser.geo stripped while a sibling user.keywords still merges normally

The three sensitive_data_*.json fixtures were regenerated. They previously encoded the wrong request shape (regs.gdpr at the top level, a root-level geo object), which is why the existing tests passed against the bug — they asserted the merger removed fields from a structure the SDK never produces.

./gradlew :PrebidMobile-core:testDebugUnitTest --tests "org.prebid.mobile.OpenRtbMergerTest"

14/14 pass. The rendering.networking.parameters package was also run as a regression check; the two failures in BasicParameterBuilderTest.whenSetPluginRendererList* are pre-existing on master and unrelated to this change.

Doc comments

Regs.getJsonObject carried "When you add a new field to this list, don't forget to add it to the OpenRtbMerger". Since regs fields now split across two lists, it names both (FIELDS_REGS for top-level, FIELDS_REGS_EXT for ext). The identical comment on geo/Geo.java had become misleading — a new Geo field needs no merger change at all now — so it explains that the object is protected by its parent instead. Device.java's comment is still accurate and untouched.

🤖 Generated with Claude Code

OpenRtbMerger.removeSensitiveData applied several field lists at the wrong
nesting level, so a publisher-supplied global ORTB config could overwrite
consent signals the SDK computes:

- FIELDS_REGS was applied to "regs", but UserConsentParameterBuilder writes
  GDPR and US Privacy into "regs.ext". Only "coppa" was actually protected;
  {"regs":{"ext":{"gdpr":0}}} silently overrode the SDK's computed value.
- FIELDS_GEO was applied to a root-level "geo" object that does not exist in
  OpenRTB, so the list never matched anything. "user.geo" was unprotected.
- "regs.gpp" and "regs.gpp_sid" are serialized by Regs but were not protected.

Each list is now applied where the SDK actually writes the field, mirroring
ArbitraryGlobalORTBHelper.ProtectedFields on iOS: separate regs/regs.ext and
user/user.ext lists. "regs.ext.tfua" stays open deliberately, since the global
ORTB config is the only way for publishers to send it (#997).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant