mesos git commit: Rejected libprocess HTTP requests with empty path.

[al...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master d345ebccf -> 1da4e45b8


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/1da4e45b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1da4e45b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1da4e45b

Branch: refs/heads/master
Commit: 1da4e45b8077b9046bba1a7ed15be5e344a14a91
Parents: d345ebc
Author: Alexander Rukletsov <al...@apache.org>
Authored: Tue Jul 4 15:24:17 2017 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Mon Jul 10 09:33:00 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/1da4e45b/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 97f737b..f42af15 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -825,6 +825,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;
@@ -2875,6 +2880,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)) {
@@ -2961,22 +2986,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) {