Skip to content

Commit

Permalink
Prevents reflection of user input when reporting URI validation error…
Browse files Browse the repository at this point in the history
…s as part of 400 response.
  • Loading branch information
spericas committed Feb 18, 2025
1 parent c325930 commit b85e42b
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates.
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -38,6 +38,10 @@ public class UriValidationException extends IllegalArgumentException {
* The value (containing illegal characters) that failed validation.
*/
private final char[] invalidValue;
/**
* The index of the invalid char.
*/
private final int index;

/**
* Create a new validation exception that uses a descriptive message and the failed chars.
Expand All @@ -52,6 +56,7 @@ public UriValidationException(Segment segment, char[] invalidValue, String messa

this.segment = segment;
this.invalidValue = invalidValue;
this.index = -1;
}

/**
Expand All @@ -67,6 +72,7 @@ public UriValidationException(Segment segment, char[] invalidValue, String messa

this.segment = segment;
this.invalidValue = invalidValue;
this.index = -1;
}

/**
Expand All @@ -84,6 +90,7 @@ public UriValidationException(Segment segment, char[] invalidValue, String messa

this.segment = segment;
this.invalidValue = invalidValue;
this.index = index;
}

/**
Expand All @@ -100,6 +107,7 @@ public UriValidationException(Segment segment, char[] invalidValue, String messa

this.segment = segment;
this.invalidValue = invalidValue;
this.index = index;
}

/**
Expand All @@ -121,6 +129,16 @@ public Segment segment() {
return segment;
}

/**
* Returns a safe message that does not include any user input and can be returned
* as part of a response.
*
* @return a safe message
*/
public String safeMessage() {
return segment.text() + " contains invalid char" + (index != -1 ? " at index " + index : "");
}

private static String toMessage(char[] value, String message) {
Objects.requireNonNull(value);
Objects.requireNonNull(message);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 Oracle and/or its affiliates.
* Copyright (c) 2024, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -76,9 +76,8 @@ void testBadQuery() {

assertThat(response, containsString("400 Bad Request"));
// beginning of message to the first double quote
assertThat(response, containsString("Query contains invalid char: "));
assertThat(response, containsString("Query contains invalid char at index 2"));
// end of message from double quote, index of bad char, and bad char
assertThat(response, containsString(", index: 2, char: 0x3c"));
assertThat(response, not(containsString(">")));
}

Expand All @@ -91,9 +90,8 @@ void testBadQueryCurly() {

assertThat(response, containsString("400 Bad Request"));
// beginning of message to the first double quote
assertThat(response, containsString("Query contains invalid char: "));
assertThat(response, containsString("Query contains invalid char at index 10"));
// end of message from double quote, index of bad char, and bad char
assertThat(response, containsString(", index: 10, char: 0x7b"));
}

@Test
Expand All @@ -117,9 +115,8 @@ void testBadFragment() {

assertThat(response, containsString("400 Bad Request"));
// beginning of message to the first double quote
assertThat(response, containsString("Fragment contains invalid char: "));
assertThat(response, containsString("Fragment contains invalid char at index 16"));
// end of message from double quote, index of bad char, and bad char
assertThat(response, containsString(", index: 16, char: 0x3e"));
assertThat(response, not(containsString(">")));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, 2024 Oracle and/or its affiliates.
* Copyright (c) 2022, 2025 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -37,6 +37,7 @@
import io.helidon.common.mapper.MapperException;
import io.helidon.common.task.InterruptableTask;
import io.helidon.common.tls.TlsUtils;
import io.helidon.common.uri.UriValidationException;
import io.helidon.common.uri.UriValidator;
import io.helidon.http.BadRequestException;
import io.helidon.http.DateTime;
Expand Down Expand Up @@ -335,7 +336,7 @@ private void validatePrologue(HttpPrologue prologue) {
.status(Status.BAD_REQUEST_400)
.request(DirectTransportRequest.create(prologue, ServerRequestHeaders.create()))
.setKeepAlive(false)
.message(e.getMessage())
.message(e instanceof UriValidationException ve ? ve.safeMessage() : e.getMessage())
.safeMessage(true)
.cause(e)
.build();
Expand Down

0 comments on commit b85e42b

Please sign in to comment.