Secure Markup Validation Update

docs/config-app.md

@@ -99,6 +99,7 @@ Removes and downloads file again if depending service cant process probably corr
 - `auction.enforce-random-bid-id` - whether to enforce generating a robust random seatbid[].bid[].id in the OpenRTB response if the initial value is less than 17 characters.
 - `auction.validations.banner-creative-max-size` - enables creative max size validation for banners. Possible values: `skip`, `enforce`, `warn`. Default is `skip`.
 - `auction.validations.secure-markup` - enables secure markup validation. Possible values: `skip`, `enforce`, `warn`. Default is `skip`.
+- `auction.validations.secure-markup-allowed-paths` - enables secure markup validation. Possible values: `skip`, `enforce`, `warn`. Default is `skip`.
 - `auction.host-schain-node` - defines global schain node that will be appended to `request.source.ext.schain.nodes` passed to bidders
 - `auction.category-mapping-enabled` - if equals to `true` the category mapping feature will be active while auction.
 - `auction.strict-app-site-dooh` - if set to `true`, it will reject requests that contain more than one of app/site/dooh. Defaults to `false`.

src/main/java/org/prebid/server/spring/config/ServiceConfiguration.java

@@ -117,6 +117,7 @@
 import org.prebid.server.validation.BidderParamValidator;
 import org.prebid.server.validation.ImpValidator;
 import org.prebid.server.validation.RequestValidator;
+import org.prebid.server.validation.ResponseBidAdmValidator;
 import org.prebid.server.validation.ResponseBidValidator;
 import org.prebid.server.validation.VideoRequestValidator;
 import org.prebid.server.vast.VastModifier;
@@ -1072,17 +1073,26 @@ BidderParamValidator bidderParamValidator(BidderCatalog bidderCatalog, JacksonMa
 
     @Bean
     ResponseBidValidator responseValidator(
+            ResponseBidAdmValidator responseBidAdmValidator,
             @Value("${auction.validations.banner-creative-max-size}") BidValidationEnforcement bannerMaxSizeEnforcement,
             @Value("${auction.validations.secure-markup}") BidValidationEnforcement secureMarkupEnforcement,
             Metrics metrics) {
 
         return new ResponseBidValidator(
+                responseBidAdmValidator,
                 bannerMaxSizeEnforcement,
                 secureMarkupEnforcement,
                 metrics,
                 logSamplingRate);
     }
 
+    @Bean
+    ResponseBidAdmValidator responseBidAdmValidator(
+            @Value("${auction.validations.secure-markup-allowed-paths:null}") Set<String> allowedPaths) {
+
+        return new ResponseBidAdmValidator(allowedPaths);
+    }
+
     @Bean
     CriteriaLogManager criteriaLogManager(JacksonMapper mapper) {
         return new CriteriaLogManager(mapper);

src/main/java/org/prebid/server/validation/ResponseBidAdmValidator.java

@@ -0,0 +1,75 @@
+package org.prebid.server.validation;
+
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+public class ResponseBidAdmValidator {
+
+    private static final String[] SECURE_MARKUP_MARKERS = {"https:", "https%3A"};
+
+    private final Collection<String> allowedPaths;
+
+    public ResponseBidAdmValidator(Collection<String> allowedPaths) {
+        this.allowedPaths = CollectionUtils.emptyIfNull(allowedPaths);
+    }
+
+    public boolean isSecure(String adm) {
+        if (!StringUtils.containsAny(adm, SECURE_MARKUP_MARKERS)) {
+            return false;
+        }
+
+        final Set<String> encodedAllowedPaths = allowedPaths.stream()
+                .map(pattern -> URLEncoder.encode(pattern, StandardCharsets.UTF_8))
+                .collect(Collectors.toSet());
+
+        return allUrlsAllowed(adm, allowedPaths, "http:", "//")
+                && allUrlsAllowed(adm, encodedAllowedPaths, "http%3A", "%2F%2F");
+    }
+
+    private static boolean allUrlsAllowed(String adm,
+                                          Collection<String> allowedPaths,
+                                          String protocol,
+                                          String doubleSlash) {
+
+        int searchStartIndex = 0;
+        while (searchStartIndex < adm.length()) {
+            final int httpIndex = adm.indexOf(protocol, searchStartIndex);
+
+            if (httpIndex == -1) {
+                return true;
+            }
+
+            final int afterHttpPrefixIndex = httpIndex + protocol.length();
+            if (afterHttpPrefixIndex + 1 > adm.length()) {
+                return true;
+            }
+
+            if (!adm.startsWith(doubleSlash, afterHttpPrefixIndex)) {
+                searchStartIndex = httpIndex + 1;
+                continue;
+            }
+
+            final int afterHttpDoubleSlashIndex = afterHttpPrefixIndex + doubleSlash.length();
+            if (afterHttpDoubleSlashIndex + 1 > adm.length()) {
+                return true;
+            }
+
+            final boolean isAllowedExactPathMatch = allowedPaths.stream()
+                    .anyMatch(allowedPattern -> adm.startsWith(allowedPattern, afterHttpDoubleSlashIndex));
+
+            if (!isAllowedExactPathMatch) {
+                return false;
+            }
+
+            searchStartIndex = httpIndex + 1;
+        }
+
+        return false;
+    }
+}

src/main/java/org/prebid/server/validation/ResponseBidValidator.java

@@ -48,20 +48,20 @@ public class ResponseBidValidator {
     private static final ConditionalLogger alternateBidderCodeLogger =
             new ConditionalLogger("alternate_bidder_code_validation", logger);
 
-    private static final String[] INSECURE_MARKUP_MARKERS = {"http:", "http%3A"};
-    private static final String[] SECURE_MARKUP_MARKERS = {"https:", "https%3A"};
-
+    private final ResponseBidAdmValidator admValidator;
     private final BidValidationEnforcement bannerMaxSizeEnforcement;
     private final BidValidationEnforcement secureMarkupEnforcement;
     private final Metrics metrics;
 
     private final double logSamplingRate;
 
-    public ResponseBidValidator(BidValidationEnforcement bannerMaxSizeEnforcement,
+    public ResponseBidValidator(ResponseBidAdmValidator admValidator,
+                                BidValidationEnforcement bannerMaxSizeEnforcement,
                                 BidValidationEnforcement secureMarkupEnforcement,
                                 Metrics metrics,
                                 double logSamplingRate) {
 
+        this.admValidator = admValidator;
         this.bannerMaxSizeEnforcement = Objects.requireNonNull(bannerMaxSizeEnforcement);
         this.secureMarkupEnforcement = Objects.requireNonNull(secureMarkupEnforcement);
         this.metrics = Objects.requireNonNull(metrics);
@@ -277,7 +277,7 @@ private List<String> validateSecureMarkup(BidderBid bidderBid,
         final Bid bid = bidderBid.getBid();
         final String adm = bid.getAdm();
 
-        if (isImpSecure(correspondingImp) && markupIsNotSecure(adm)) {
+        if (isImpSecure(correspondingImp) && !admValidator.isSecure(adm)) {
             final String message = """
                     BidResponse validation `%s`: bidder `%s` response triggers secure \
                     creative validation for bid %s, account=%s, referrer=%s, adm=%s"""
@@ -301,11 +301,6 @@ private static boolean isImpSecure(Imp imp) {
         return Objects.equals(imp.getSecure(), 1);
     }
 
-    private static boolean markupIsNotSecure(String adm) {
-        return StringUtils.containsAny(adm, INSECURE_MARKUP_MARKERS)
-                || !StringUtils.containsAny(adm, SECURE_MARKUP_MARKERS);
-    }
-
     private List<String> singleWarningOrValidationException(
             BidValidationEnforcement enforcement,
             Consumer<MetricName> metricsRecorder,

src/test/java/org/prebid/server/validation/ResponseBidAdmValidatorTest.java

@@ -0,0 +1,77 @@
+package org.prebid.server.validation;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class ResponseBidAdmValidatorTest {
+
+    //todo: add separate test for unique cases
+    private final ResponseBidAdmValidator target = new ResponseBidAdmValidator(Set.of("www.w3.org", "www.url.ua/page"));
+
+    @Test
+    public void shouldValidateAdm() {
+        assertThat(target.isSecure("http:")).isFalse();
+        assertThat(target.isSecure("http:http:")).isFalse();
+        // treats 'https' as a domain, which is not allowed
+        assertThat(target.isSecure("http://https:")).isFalse();
+        // 'http:///' is actually empty
+        assertThat(target.isSecure("https:http:///")).isFalse();
+        assertThat(target.isSecure("http://www.url.uahttps:")).isFalse();
+
+        assertThat(target.isSecure("https:")).isTrue();
+        assertThat(target.isSecure("http:https:")).isTrue();
+        assertThat(target.isSecure("http:/https:")).isTrue();
+        assertThat(target.isSecure("http:https://")).isTrue();
+        // 'http://' is treated as empty only in the end
+        assertThat(target.isSecure("https:http://")).isTrue();
+        assertThat(target.isSecure("https:http:")).isTrue();
+        assertThat(target.isSecure("https:https:")).isTrue();
+        assertThat(target.isSecure("http://www.w3.orghttps:")).isTrue();
+        assertThat(target.isSecure("http://www.url.ua/pagehttps:")).isTrue();
+        // empty domain because no slashes
+        assertThat(target.isSecure("http:www.w3.orghttps:")).isTrue();
+    }
+
+    @Test
+    public void shouldValidateUrlEncodedAdm() {
+        // case insensitive
+        assertThat(target.isSecure("https%3a")).isFalse();
+        assertThat(target.isSecure("http%3A")).isFalse();
+        assertThat(target.isSecure("http%3Ahttp%3A")).isFalse();
+        //treats 'https' as a domain, which is not allowed
+        assertThat(target.isSecure("http%3A%2F%2Fhttps%3A")).isFalse();
+        assertThat(target.isSecure("https%3Ahttp%3A%2F%2F%2F")).isFalse();
+        assertThat(target.isSecure("http%3A%2F%2Fwww.url.uahttps%3A")).isFalse();
+
+        assertThat(target.isSecure("https%3A")).isTrue();
+        assertThat(target.isSecure("http%3Ahttps%3A")).isTrue();
+        assertThat(target.isSecure("http%3A%2Fhttps%3A")).isTrue();
+        assertThat(target.isSecure("http%3Ahttps%3A%2F%2F")).isTrue();
+        // http:// is treated as empty only in the end
+        assertThat(target.isSecure("https%3Ahttp%3A%2F%2F")).isTrue();
+        assertThat(target.isSecure("https%3Ahttps%3A")).isTrue();
+        assertThat(target.isSecure("http%3A%2F%2Fwww.w3.orghttps%3A")).isTrue();
+        assertThat(target.isSecure("http%3A%2F%2Fwww.url.ua%2Fpagehttps%3A")).isTrue();
+        // empty domain because no slashes
+        assertThat(target.isSecure("http%3Awww.w3.orghttps%3A")).isTrue();
+    }
+
+    @Test
+    public void shouldValidatePlainAndUrlEncodedPaths() {
+        // combined plain and encoded is not allowed
+        assertThat(target.isSecure("http://www.url.ua%2Fpagehttps%3A")).isFalse();
+
+        assertThat(target.isSecure("http%3Ahttps:")).isTrue();
+        assertThat(target.isSecure("http:https%3A")).isTrue();
+        assertThat(target.isSecure("https%3Ahttp%3A//")).isTrue();
+        // 'http%3A//' is treated like an empty 'http:' and '//' separately
+        assertThat(target.isSecure("http%3A//any.domainhttps%3A")).isTrue();
+        assertThat(target.isSecure("http://www.url.ua/pagehttps%3A")).isTrue();
+        // empty domain because no slashes
+        assertThat(target.isSecure("http:www.w3.orghttps%3A")).isTrue();
+        assertThat(target.isSecure("http://www.w3.orghttps://any.domain,http%3Aanything")).isTrue();
+    }
+}