Merge pull request #42 from curityio/wp-fix-jwt-pointer-arithmetic

fix memcpy fixes #42
ToucanToco · Nov 29, 2018 · d637340 · d637340
2 parents d067019 + bbf6ed3
commit d637340
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 17 deletions.
diff --git a/configure b/configure
@@ -31,7 +31,7 @@ if [[ -z "${NGINX_DEBUG+xxx}" ]]; then
 fi      
 
 if [[ "$NGINX_DEBUG" =~ ^([yY][eE][sS]|[yY])+$ ]]; then
-    CONFIG_OPTS+=(--with-debug --with-cc-opt="-O0 -g")
+    CONFIG_OPTS+=(--with-debug --with-cc-opt="-O0 -g3")
 else
     CONFIG_OPTS+=(--with-cc-opt="-DNDEBUG")
 fi

diff --git a/phantom_token.c b/phantom_token.c
@@ -424,7 +424,7 @@ static ngx_int_t handler(ngx_http_request_t *request)
     introspection_request->request_body = introspection_request_body;
     introspection_request->headers_in.content_length_n = ngx_buf_size(introspection_request_body_buffer);
 
-#if(NGX_HTTP_HEADERS)
+#if (NGX_HTTP_HEADERS)
     if (request->headers_in.accept == NULL)
     {
         ngx_int_t result;
@@ -628,54 +628,68 @@ static ngx_int_t introspection_response_handler(ngx_http_request_t *request, voi
         return introspection_subrequest_status_code;
     }
 
-    // body parsing
     u_char *jwt_start = NULL;
+    ngx_str_t cache_data = ngx_null_string;
 
+#if (NGX_HTTP_CACHE)
     if (!request->cache || !request->cache->buf)
     {
+        // No cache; read JWT from response to sub-request
         jwt_start = request->header_end + sizeof("\r\n") - 1;
     }
 
     if (jwt_start == NULL && request->cache && request->cache->buf && request->cache->valid_sec > 0)
     {
-        ngx_read_file(&request->cache->file, request->cache->buf->pos, request->cache->length, 0);
+        // Try to read JWT from cache
 
-        jwt_start = request->cache->buf->start + request->cache->body_start;
+        cache_data.len = request->cache->length;
+        cache_data.data = ngx_pnalloc(request->pool, cache_data.len);
+
+        if (cache_data.data != NULL)
+        {
+            ngx_read_file(&request->cache->file, cache_data.data, cache_data.len, request->cache->body_start);
+
+            jwt_start = cache_data.data;
+        }
     }
 
     if (jwt_start == NULL)
     {
-        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, request->connection->log, 0, "Failed to parse response");
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, request->connection->log, 0,
+                       "Failed to obtain JWT from introspection response or, if applicable, cache");
+
         module_context->done = 1;
         module_context->status = NGX_HTTP_UNAUTHORIZED;
 
         return introspection_subrequest_status_code;
     }
+#else
+    jwt_start = request->header_end + sizeof("\r\n") - 1;
+#endif
+
+    size_t jwt_len = request->headers_out.content_length_n;
+    size_t bearer_jwt_len = BEARER_SIZE + jwt_len;
 
-    u_char *jwt_end = jwt_start + request->headers_out.content_length_n;
+    module_context->jwt.len = bearer_jwt_len;
+    module_context->jwt.data = ngx_pnalloc(request->pool, bearer_jwt_len);
 
-    if (jwt_end == NULL)
+    if (module_context->jwt.data == NULL)
     {
-        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, request->connection->log, 0, "Failed to parse response");
         module_context->done = 1;
         module_context->status = NGX_HTTP_UNAUTHORIZED;
 
         return introspection_subrequest_status_code;
     }
 
-    module_context->jwt.len = jwt_end - jwt_start + BEARER_SIZE;
+    u_char *p = ngx_copy(module_context->jwt.data, BEARER, BEARER_SIZE);
 
-    module_context->jwt.data = ngx_pcalloc(request->pool, module_context->jwt.len);
+    ngx_memcpy(p, jwt_start, jwt_len);
 
-    if (module_context->jwt.data == NULL)
+    if (cache_data.len > 0)
     {
-        return introspection_subrequest_status_code;
+        ngx_pfree(request->pool, cache_data.data);
     }
 
-    u_char *p = ngx_copy(module_context->jwt.data, BEARER, BEARER_SIZE);
-
-    ngx_memcpy(p, jwt_start, module_context->jwt.len);
-
     module_context->done = 1;
 
     return introspection_subrequest_status_code;