mesos-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From al...@apache.org
Subject mesos git commit: Rejected libprocess HTTP requests with empty path.
Date Mon, 10 Jul 2017 08:46:17 GMT
Repository: mesos
Updated Branches:
  refs/heads/1.3.x 716b81a02 -> e27e32d1d


Rejected libprocess HTTP requests with empty path.

Without this patch, a malicious actor can crash libprocess-based
components by sending a libprocess HTTP message with empty path.

For robustness, we check for malformed HTTP requests in both
handle() and parse() routines in libprocess, because there is
no guarantee that parse() will always get a validated request.

A better approach would be to introduce an explicit HTTP request
validation stage, for both libprocess and common HTTP messages.


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e27e32d1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e27e32d1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e27e32d1

Branch: refs/heads/1.3.x
Commit: e27e32d1ddb0d5cfc2bcdaea4e8adf9e350963d9
Parents: 716b81a
Author: Alexander Rukletsov <alexr@apache.org>
Authored: Tue Jul 4 15:24:17 2017 +0200
Committer: Alexander Rukletsov <alexr@apache.org>
Committed: Mon Jul 10 10:45:54 2017 +0200

----------------------------------------------------------------------
 3rdparty/libprocess/src/process.cpp | 42 ++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e27e32d1/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index ffac010..dac4802 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -788,6 +788,11 @@ static Future<Message*> parse(const Request& request)
     return Failure("Failed to determine sender from request headers");
   }
 
+  // Check that URL path is present and starts with '/'.
+  if (request.url.path.find('/') != 0) {
+    return Failure("Request URL path must start with '/'");
+  }
+
   // Now determine 'to'.
   size_t index = request.url.path.find('/', 1);
   index = index != string::npos ? index - 1 : string::npos;
@@ -2826,6 +2831,26 @@ void ProcessManager::handle(
 {
   CHECK(request != nullptr);
 
+  // Start by checking that the path starts with a '/'.
+  if (request->url.path.find('/') != 0) {
+    VLOG(1) << "Returning '400 Bad Request' for '" << request->url.path <<
"'";
+
+    // Get the HttpProxy pid for this socket.
+    PID<HttpProxy> proxy = socket_manager->proxy(socket);
+
+    // Enqueue the response with the HttpProxy so that it respects the
+    // order of requests to account for HTTP/1.1 pipelining.
+    dispatch(
+        proxy,
+        &HttpProxy::enqueue,
+        BadRequest("Request URL path must start with '/'"),
+        *request);
+
+    // Cleanup request.
+    delete request;
+    return;
+  }
+
   // Check if this is a libprocess request (i.e., 'User-Agent:
   // libprocess/id@ip:port') and if so, parse as a message.
   if (libprocess(request)) {
@@ -2883,22 +2908,7 @@ void ProcessManager::handle(
     return;
   }
 
-  // Treat this as an HTTP request. Start by checking that the path
-  // starts with a '/' (since the code below assumes as much).
-  if (request->url.path.find('/') != 0) {
-    VLOG(1) << "Returning '400 Bad Request' for '" << request->url.path <<
"'";
-
-    // Get the HttpProxy pid for this socket.
-    PID<HttpProxy> proxy = socket_manager->proxy(socket);
-
-    // Enqueue the response with the HttpProxy so that it respects the
-    // order of requests to account for HTTP/1.1 pipelining.
-    dispatch(proxy, &HttpProxy::enqueue, BadRequest(), *request);
-
-    // Cleanup request.
-    delete request;
-    return;
-  }
+  // Treat this as an HTTP request.
 
   // Ignore requests with relative paths (i.e., contain "/..").
   if (request->url.path.find("/..") != string::npos) {


Mime
View raw message