diff --git a/src/main/java/com/retailsvc/http/Request.java b/src/main/java/com/retailsvc/http/Request.java index 13d0404..545c878 100644 --- a/src/main/java/com/retailsvc/http/Request.java +++ b/src/main/java/com/retailsvc/http/Request.java @@ -1,11 +1,9 @@ package com.retailsvc.http; +import com.retailsvc.http.internal.QueryParams; import com.retailsvc.http.spec.HttpMethod; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -265,7 +263,7 @@ public String rawQuery() { */ public Map queryParams() { if (queryParamCache == null) { - queryParamCache = parseQuery(rawQuery); + queryParamCache = QueryParams.parse(rawQuery); } return queryParamCache; } @@ -345,23 +343,4 @@ public void afterResponse(Runnable runnable) { public List afterHooks() { return Collections.unmodifiableList(afterHooks); } - - private static Map parseQuery(String query) { - if (query == null || query.isBlank()) { - return Map.of(); - } - Map out = new LinkedHashMap<>(); - for (String pair : query.split("&")) { - if (pair.isEmpty()) { - continue; - } - int eq = pair.indexOf('='); - String rawKey = eq < 0 ? pair : pair.substring(0, eq); - String rawValue = eq < 0 ? "" : pair.substring(eq + 1); - out.putIfAbsent( - URLDecoder.decode(rawKey, StandardCharsets.UTF_8), - URLDecoder.decode(rawValue, StandardCharsets.UTF_8)); - } - return out; - } } diff --git a/src/main/java/com/retailsvc/http/internal/QueryParams.java b/src/main/java/com/retailsvc/http/internal/QueryParams.java new file mode 100644 index 0000000..40f10af --- /dev/null +++ b/src/main/java/com/retailsvc/http/internal/QueryParams.java @@ -0,0 +1,42 @@ +package com.retailsvc.http.internal; + +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Parses a raw (percent-encoded) URI query string into decoded parameters. Splitting on {@code &} + * and {@code =} happens before per-component decoding, so a percent-encoded separator + * ({@code %26}, {@code %3D}) stays inside a value instead of becoming a delimiter. Request + * validation and handler consumption both parse through here, so the value validated is exactly the + * value the handler receives. + */ +public final class QueryParams { + + private QueryParams() {} + + /** + * Parses {@code rawQuery} (the undecoded query component, e.g. {@code URI.getRawQuery()}) into a + * name→value map. Values are URL-decoded with UTF-8. For repeated names the first occurrence + * wins. + */ + public static Map parse(String rawQuery) { + if (rawQuery == null || rawQuery.isBlank()) { + return Map.of(); + } + Map out = new LinkedHashMap<>(); + for (String pair : rawQuery.split("&")) { + if (pair.isEmpty()) { + continue; + } + int eq = pair.indexOf('='); + String rawKey = eq < 0 ? pair : pair.substring(0, eq); + String rawValue = eq < 0 ? "" : pair.substring(eq + 1); + out.putIfAbsent( + URLDecoder.decode(rawKey, StandardCharsets.UTF_8), + URLDecoder.decode(rawValue, StandardCharsets.UTF_8)); + } + return out; + } +} diff --git a/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java b/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java index f9283dc..68014e0 100644 --- a/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java +++ b/src/main/java/com/retailsvc/http/internal/RequestPreparationFilter.java @@ -20,7 +20,6 @@ import com.sun.net.httpserver.Headers; import com.sun.net.httpserver.HttpExchange; import java.io.IOException; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -189,7 +188,7 @@ private void validateParameters( for (Parameter p : op.parameters()) { String pointer = p.pointer(); if (p.in() == Parameter.Location.QUERY && query == null) { - query = parseQuery(exchange.getRequestURI().getQuery()); + query = QueryParams.parse(exchange.getRequestURI().getRawQuery()); } String value = switch (p.in()) { @@ -253,19 +252,4 @@ private ParsedBody validateAndParseBody(HttpExchange exchange, Operation op, byt validator.validate(parsed, mt.schema(), ""); return new ParsedBody(parsed, mapper); } - - private static Map parseQuery(String query) { - if (query == null || query.isBlank()) { - return Map.of(); - } - Map out = new HashMap<>(); - for (String pair : query.split("&")) { - int eq = pair.indexOf('='); - if (eq <= 0) { - continue; - } - out.putIfAbsent(pair.substring(0, eq), pair.substring(eq + 1)); - } - return out; - } } diff --git a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java index 6490210..25de7d1 100644 --- a/src/main/java/com/retailsvc/http/validate/DefaultValidator.java +++ b/src/main/java/com/retailsvc/http/validate/DefaultValidator.java @@ -21,6 +21,7 @@ import com.retailsvc.http.spec.schema.StringSchema; import com.retailsvc.http.spec.schema.TypeName; import java.math.BigDecimal; +import java.math.BigInteger; import java.net.URI; import java.net.URISyntaxException; import java.time.LocalDate; @@ -47,6 +48,8 @@ public final class DefaultValidator implements Validator { private static final String FORMAT_KEYWORD = "format"; + private static final String MINIMUM_KEYWORD = "minimum"; + private static final String MAXIMUM_KEYWORD = "maximum"; private static final Optional OK = Optional.empty(); private record FormatCheck(Predicate isValid, String message) {} @@ -365,18 +368,39 @@ private static boolean isHextet(String hextet) { return true; } + /** Largest magnitude an IEEE-754 double represents as an exact integer (2^53). */ private static Optional checkInteger( Object value, IntegerSchema s, String pointer) { if (!(value instanceof Number num)) { return err(pointer, "type", "expected integer", value); } - long n = num.longValue(); + // Fast path: integral wrappers bound-check as a long with no allocation. This is the common + // case (e.g. Jackson int/long, Gson integral tokens parsed as Long). + if (num instanceof Long + || num instanceof Integer + || num instanceof Short + || num instanceof Byte) { + return checkIntegerBounds(num.longValue(), s, pointer); + } + // Any other numeric type (Double, BigDecimal, ...) must be a whole number: convert exactly and + // reject a fractional part (e.g. 4.9) or a non-finite value, comparing the full magnitude so + // nothing wraps past a bound. + BigInteger n; + try { + n = new BigDecimal(num.toString()).toBigIntegerExact(); + } catch (ArithmeticException | NumberFormatException _) { + return err(pointer, "type", "expected integer", value); + } + return checkIntegerBounds(n, s, pointer); + } + private static Optional checkIntegerBounds( + long n, IntegerSchema s, String pointer) { if (s.minimum() != null && n < s.minimum()) { - return err(pointer, "minimum", "integer below minimum " + s.minimum(), n); + return err(pointer, MINIMUM_KEYWORD, "integer below minimum " + s.minimum(), n); } if (s.maximum() != null && n > s.maximum()) { - return err(pointer, "maximum", "integer above maximum " + s.maximum(), n); + return err(pointer, MAXIMUM_KEYWORD, "integer above maximum " + s.maximum(), n); } if (s.exclusiveMinimum() != null && n <= s.exclusiveMinimum()) { return err( @@ -394,6 +418,25 @@ private static Optional checkInteger( return OK; } + private static Optional checkIntegerBounds( + BigInteger n, IntegerSchema s, String pointer) { + if (n.bitLength() < Long.SIZE) { + return checkIntegerBounds(n.longValue(), s, pointer); + } + // Magnitude exceeds signed-long range: it breaches whichever bound lies on its side, and no + // int32/int64 format can represent it. + if (n.signum() > 0 && (s.maximum() != null || s.exclusiveMaximum() != null)) { + return err(pointer, MAXIMUM_KEYWORD, "integer out of range", n); + } + if (n.signum() < 0 && (s.minimum() != null || s.exclusiveMinimum() != null)) { + return err(pointer, MINIMUM_KEYWORD, "integer out of range", n); + } + if (s.format() != null && INTEGER_FORMAT_CHECKS.containsKey(s.format())) { + return err(pointer, FORMAT_KEYWORD, INTEGER_FORMAT_CHECKS.get(s.format()).message(), n); + } + return OK; + } + private static Optional checkNumber( Object value, NumberSchema s, String pointer) { if (!(value instanceof Number num)) { @@ -402,10 +445,10 @@ private static Optional checkNumber( double n = num.doubleValue(); if (s.minimum() != null && n < s.minimum().doubleValue()) { - return err(pointer, "minimum", "number below minimum " + s.minimum(), n); + return err(pointer, MINIMUM_KEYWORD, "number below minimum " + s.minimum(), n); } if (s.maximum() != null && n > s.maximum().doubleValue()) { - return err(pointer, "maximum", "number above maximum " + s.maximum(), n); + return err(pointer, MAXIMUM_KEYWORD, "number above maximum " + s.maximum(), n); } if (s.exclusiveMinimum() != null && n <= s.exclusiveMinimum().doubleValue()) { return err(pointer, "exclusiveMinimum", "number not greater than " + s.exclusiveMinimum(), n); diff --git a/src/test/java/com/retailsvc/http/QueryParamSmugglingTest.java b/src/test/java/com/retailsvc/http/QueryParamSmugglingTest.java new file mode 100644 index 0000000..e8a85aa --- /dev/null +++ b/src/test/java/com/retailsvc/http/QueryParamSmugglingTest.java @@ -0,0 +1,58 @@ +package com.retailsvc.http; + +import static java.net.http.HttpRequest.BodyPublishers.noBody; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import com.retailsvc.http.spec.Spec; +import java.io.InputStream; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class QueryParamSmugglingTest extends ServerBaseTest { + + @Test + void encodedAmpersandCannotSmuggleAValuePastQueryValidation() throws Exception { + // Constrain q1 so any value containing '&' or '=' fails validation. + constrainQ1ToLowercaseLetters(); + server = newServer(Map.of()); + + // %26 must stay inside q1: the handler would decode q1 to "abc&x=y", so validation must see the + // same value and reject it against ^[a-z]+$ (400). The exploit was that validation parsed the + // already-decoded query and saw q1="abc" (passing) while the handler received "abc&x=y". + var request = newRequest(server, "/params/query?q1=abc%26x=y&q2=ok", "GET", noBody()); + HttpResponse response = httpClient().send(request, BodyHandlers.ofString()); + + assertThat(response.statusCode()).isEqualTo(400); + } + + @SuppressWarnings("unchecked") + private void constrainQ1ToLowercaseLetters() { + spec = + assertDoesNotThrow( + () -> { + try (InputStream in = + QueryParamSmugglingTest.class.getResourceAsStream("/openapi.json")) { + Map raw = + (Map) + gson.fromJson( + new String(in.readAllBytes(), StandardCharsets.UTF_8), Map.class); + var paths = (Map) raw.get("paths"); + var op = + (Map) + ((Map) paths.get("/params/query")).get("get"); + for (Object p : (List) op.get("parameters")) { + var param = (Map) p; + if ("q1".equals(param.get("name"))) { + ((Map) param.get("schema")).put("pattern", "^[a-z]+$"); + } + } + return Spec.from(raw); + } + }); + } +} diff --git a/src/test/java/com/retailsvc/http/internal/QueryParamsTest.java b/src/test/java/com/retailsvc/http/internal/QueryParamsTest.java new file mode 100644 index 0000000..74700c4 --- /dev/null +++ b/src/test/java/com/retailsvc/http/internal/QueryParamsTest.java @@ -0,0 +1,34 @@ +package com.retailsvc.http.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; + +import org.junit.jupiter.api.Test; + +class QueryParamsTest { + + @Test + void nullOrBlankQueryYieldsEmptyMap() { + assertThat(QueryParams.parse(null)).isEmpty(); + assertThat(QueryParams.parse(" ")).isEmpty(); + } + + @Test + void encodedSeparatorStaysInsideValue() { + // %26 is decoded after splitting, so it does not become a delimiter: one param, not two. + assertThat(QueryParams.parse("q=a%26b=c")).containsExactly(entry("q", "a&b=c")); + } + + @Test + void splitsPairsDecodesAndKeepsFirstOccurrence() { + assertThat(QueryParams.parse("name=Alice%20Smith&name=Bob&city=x")) + .containsExactly(entry("name", "Alice Smith"), entry("city", "x")); + } + + @Test + void skipsEmptyPairsAndHandlesKeysWithoutValue() { + // Empty pair (from "&&") is skipped; a key with no '=' maps to an empty value. + assertThat(QueryParams.parse("a=1&&flag&b=2")) + .containsExactly(entry("a", "1"), entry("flag", ""), entry("b", "2")); + } +} diff --git a/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java b/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java index ee8cc2e..4a0ee02 100644 --- a/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java +++ b/src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java @@ -8,6 +8,7 @@ import com.retailsvc.http.spec.schema.NumberSchema; import com.retailsvc.http.spec.schema.StringSchema; import com.retailsvc.http.spec.schema.TypeName; +import java.math.BigInteger; import java.util.List; import java.util.Map; import java.util.Set; @@ -225,6 +226,107 @@ void integerMultipleOf() { .isEqualTo("multipleOf"); } + @Test + void integerRejectsFractionalDouble() { + // Exploit: a Double with a fractional part must not satisfy `type: integer`. Master narrowed + // via + // longValue() before the bound check, so 4.9 slipped through minimum/maximum as 4. + IntegerSchema s = + new IntegerSchema(Set.of(TypeName.INTEGER), 1L, 5L, null, null, null, null, Map.of()); + assertThatThrownBy(() -> v.validate(4.9, s, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("type"); + } + + @Test + void integerAcceptsIntegralDouble() { + IntegerSchema s = + new IntegerSchema(Set.of(TypeName.INTEGER), 1L, 5L, null, null, null, null, Map.of()); + assertThatCode(() -> v.validate(4.0, s, "/v")).doesNotThrowAnyException(); + } + + @Test + void integerRejectsValueOverflowingLongPastMaximum() { + // Exploit: 2^64 + 50 narrows to 50 via longValue(), sneaking past maximum:1000. The full + // magnitude must be compared so the bound cannot be wrapped. + IntegerSchema s = + new IntegerSchema(Set.of(TypeName.INTEGER), 0L, 1000L, null, null, null, null, Map.of()); + BigInteger huge = BigInteger.TWO.pow(64).add(BigInteger.valueOf(50)); + assertThatThrownBy(() -> v.validate(huge, s, "/v")) + .isInstanceOf(ValidationException.class) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("maximum"); + } + + @Test + void integerExclusiveBounds() { + IntegerSchema exclMin = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, 5L, null, null, null, Map.of()); + assertThatCode(() -> v.validate(6, exclMin, "/v")).doesNotThrowAnyException(); + assertThatThrownBy(() -> v.validate(5, exclMin, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("exclusiveMinimum"); + + IntegerSchema exclMax = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, 5L, null, null, Map.of()); + assertThatCode(() -> v.validate(4, exclMax, "/v")).doesNotThrowAnyException(); + assertThatThrownBy(() -> v.validate(5, exclMax, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("exclusiveMaximum"); + } + + @Test + void integerLargeButWithinLongRangeIsCheckedPrecisely() { + // > 2^53 skips the exact-double fast path, but 2^60 still fits in a long and is bound-checked. + IntegerSchema s = + new IntegerSchema(Set.of(TypeName.INTEGER), 0L, 1000L, null, null, null, null, Map.of()); + assertThatThrownBy(() -> v.validate(BigInteger.TWO.pow(60), s, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("maximum"); + } + + @Test + void integerOverflowingLongIsCheckedAgainstBoundsAndFormat() { + BigInteger huge = BigInteger.TWO.pow(64).add(BigInteger.valueOf(50)); // > Long.MAX + BigInteger hugeNegative = huge.negate(); // < Long.MIN + + IntegerSchema max = + new IntegerSchema(Set.of(TypeName.INTEGER), 0L, 1000L, null, null, null, null, Map.of()); + assertThatThrownBy(() -> v.validate(huge, max, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("maximum"); + + IntegerSchema exclMax = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, 1000L, null, null, Map.of()); + assertThatThrownBy(() -> v.validate(huge, exclMax, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("maximum"); + + IntegerSchema min = + new IntegerSchema(Set.of(TypeName.INTEGER), 0L, null, null, null, null, null, Map.of()); + assertThatThrownBy(() -> v.validate(hugeNegative, min, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("minimum"); + + IntegerSchema exclMin = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, 0L, null, null, null, Map.of()); + assertThatThrownBy(() -> v.validate(hugeNegative, exclMin, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("minimum"); + + IntegerSchema int64 = + new IntegerSchema( + Set.of(TypeName.INTEGER), null, null, null, null, null, "int64", Map.of()); + assertThatThrownBy(() -> v.validate(huge, int64, "/v")) + .extracting(t -> ((ValidationException) t).error().keyword()) + .isEqualTo("format"); + + IntegerSchema unbounded = + new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null, Map.of()); + assertThatCode(() -> v.validate(huge, unbounded, "/v")).doesNotThrowAnyException(); + } + @Test void numberAcceptsDoublesAndIntegers() { NumberSchema s =