Skip to content

Commit

Permalink
Filter out invalid URI in the error message of no handler found for a…
Browse files Browse the repository at this point in the history
… REST request

Signed-off-by: Tianli Feng <ftianli@amazon.com>
  • Loading branch information
Tianli Feng committed Jun 1, 2022
1 parent ef4e190 commit 34f1b8f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 27 deletions.
32 changes: 9 additions & 23 deletions server/src/main/java/org/opensearch/rest/RestController.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
Expand Down Expand Up @@ -488,9 +489,14 @@ private void handleBadRequest(String uri, RestRequest.Method method, RestChannel
try (XContentBuilder builder = channel.newErrorBuilder()) {
builder.startObject();
{
// Escaping HTML special characters in the error message only aims to satisfy common security scanners.
// There is no vulnerability without escaping, because the response MIME type is application/json, no scripts will be run.
builder.field("error", "no handler found for uri [" + escapeHtml(uri) + "] and method [" + method + "]");
try {
// Validate input URI to filter out HTML special characters in the error message,
// in case false-positive cross site scripting vulnerability is detected by common security scanners.
uri = new URI(uri).getPath();
builder.field("error", "no handler found for uri [" + uri + "] and method [" + method + "]");
} catch (Exception e) {
builder.field("error", "invalid uri has been requested");
}
}
builder.endObject();
channel.sendResponse(new BytesRestResponse(BAD_REQUEST, builder));
Expand Down Expand Up @@ -580,24 +586,4 @@ private static CircuitBreaker inFlightRequestsBreaker(CircuitBreakerService circ
// We always obtain a fresh breaker to reflect changes to the breaker configuration.
return circuitBreakerService.getBreaker(CircuitBreaker.IN_FLIGHT_REQUESTS);
}

/**
* Perform an HTML escape operation on a String input to prevent XSS vulnerability.
* @param text the String to be escaped.
* @return The escaped result String.
*/
private String escapeHtml(String text) {
StringBuilder out = new StringBuilder();
for (int i = 0; i < text.length(); i++) {
char c = text.charAt(i);
if (c == '"' || c == '\'' || c == '<' || c == '>' || c == '&') {
out.append("&#");
out.append((int) c);
out.append(';');
} else {
out.append(c);
}
}
return out.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -559,10 +559,7 @@ public void testHandleBadRequestWithHtmlSpecialCharsInUri() {
).build();
final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST);
restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext());
assertThat(
channel.getRestResponse().content().utf8ToString(),
containsString("/&#60;script&#62;alert(&#39;xss&#39;);alert(&#34;&#38;#x6A&#38;#x61&#38;#x76&#38;#x61&#34;);&#60;/script&#62;")
);
assertThat(channel.getRestResponse().content().utf8ToString(), containsString("invalid uri has been requested"));
}

public void testDispatchUnsupportedHttpMethod() {
Expand Down

0 comments on commit 34f1b8f

Please sign in to comment.