honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Filter out invalid URI and HTTP method in the error message of no handler found for a REST request (#3459) 

Filter out invalid URI and HTTP method of a error message, which shown when there is no handler found for a REST request sent by user, so that HTML special characters <>&"' will not shown in the error message.

The error message is return as mine-type `application/json`, which can't contain active (script) content, so it's not a vulnerability. Besides, no browsers are going to render as html when the mine-type is that.
While the common security scanners will raise a false-positive alarm for having HTML tags in the response without escaping the HTML special characters, so the solution only aims to satisfy the code security scanners.

Signed-off-by: Tianli Feng <ftianli@amazon.com>
This commit is contained in:
Tianli Feng 2022-06-02 09:29:32 -07:00 committed by GitHub
parent 1ceff286a0
commit 2bfe8b31af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
server/src/main/java/org/opensearch/rest/RestController.java
View File

@ -56,6 +56,7 @@ import org.opensearch.usage.UsageService;
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;
@ -447,7 +448,9 @@ public class RestController implements HttpServerTransport.Dispatcher {
msg.append("Incorrect HTTP method for uri [").append(uri);
msg.append("] and method [").append(method).append("]");
} else {
msg.append(exception.getMessage());
// Not using the error message directly from 'exception.getMessage()' to avoid unescaped HTML special characters,
// in case false-positive cross site scripting vulnerability is detected by common security scanners.
msg.append("Unexpected HTTP method");
}
if (validMethodSet.isEmpty() == false) {
msg.append(", allowed: ").append(validMethodSet);
@ -488,7 +491,14 @@ public class RestController implements HttpServerTransport.Dispatcher {
try (XContentBuilder builder = channel.newErrorBuilder()) {
builder.startObject();
{
builder.field("error", "no handler found for uri [" + 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));

13
server/src/test/java/org/opensearch/rest/RestControllerTests.java
View File

@ -553,6 +553,15 @@ public class RestControllerTests extends OpenSearchTestCase {
assertThat(channel.getRestResponse().getHeaders().get("Allow"), hasItem(equalTo(RestRequest.Method.GET.toString())));
}
public void testHandleBadRequestWithHtmlSpecialCharsInUri() {
final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withPath(
"/<script>alert('xss');alert(\"&#x6A&#x61&#x76&#x61\");</script>"
).build();
final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST);
restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext());
assertThat(channel.getRestResponse().content().utf8ToString(), containsString("invalid uri has been requested"));
}
public void testDispatchUnsupportedHttpMethod() {
final boolean hasContent = randomBoolean();
final RestRequest request = RestRequest.request(xContentRegistry(), new HttpRequest() {
@ -623,6 +632,10 @@ public class RestControllerTests extends OpenSearchTestCase {
assertTrue(channel.getSendResponseCalled());
assertThat(channel.getRestResponse().getHeaders().containsKey("Allow"), equalTo(true));
assertThat(channel.getRestResponse().getHeaders().get("Allow"), hasItem(equalTo(RestRequest.Method.GET.toString())));
assertThat(
channel.getRestResponse().content().utf8ToString(),
equalTo("{\"error\":\"Unexpected HTTP method, allowed: [GET]\",\"status\":405}")
);
}
private static final class TestHttpServerTransport extends AbstractLifecycleComponent implements HttpServerTransport {