Prevent derefencing NULL pointer when OCSP body contains no nextUpdate element

[Baldwin, Matthew]

Hi!

Thanks, I think that makes sense.

Matthew

-----Original Message-----
From: nginx-devel [mailto:nginx-devel-bounces at nginx.org] On Behalf Of Maxim Dounin
Sent: Monday, July 13, 2015 10:35 AM
To: nginx-devel at nginx.org
Subject: Re: Prevent derefencing NULL pointer when OCSP body contains no nextUpdate element

Hello!

On Thu, Jul 09, 2015 at 06:53:12PM +0000, Baldwin, Matthew wrote:

> Hi!
> 
> If nextUpdate is NULL when processing an OCSP response, nginx will 
> core with SIGSEGV in ngx_ssl_stapling_time when calling 
> ASN1_GENERALIZEDTIME_print
> 
> The following patch against nginx-1.9.2 prevents this:

Thanks for the report.  It looks like at least RFC 6960 allows OCSP responses without nextUpdate, so I would suggest something like this to handle such responses instead:

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru> # Date 1436808659 -10800
#      Mon Jul 13 20:30:59 2015 +0300
# Node ID 92b6f9dd1e7a66a5b34987b9e637450b4a3d8f37
# Parent  dcae651b2a0cbd3de2f1fd5cf5b8c72627db94fd
OCSP stapling: fixed segfault without nextUpdate.

OCSP responses may not contain nextUpdate.  As per RFC 6960, this means that nextUpdate checks should be bypassed.  Handle this gracefully by using NGX_MAX_TIME_T_VALUE as "valid" in such a case.

The problem was introduced by 6893a1007a7c (1.9.2).

Reported by Matthew Baldwin.

diff --git a/src/event/ngx_event_openssl_stapling.c b/src/event/ngx_event_openssl_stapling.c
--- a/src/event/ngx_event_openssl_stapling.c
+++ b/src/event/ngx_event_openssl_stapling.c
@@ -637,11 +637,16 @@ ngx_ssl_stapling_ocsp_handler(ngx_ssl_oc
         goto error;
     }
 
-    valid = ngx_ssl_stapling_time(nextupdate);
-    if (valid == (time_t) NGX_ERROR) {
-        ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
-                      "invalid nextUpdate time in certificate status");
-        goto error;
+    if (nextupdate) {
+        valid = ngx_ssl_stapling_time(nextupdate);
+        if (valid == (time_t) NGX_ERROR) {
+            ngx_log_error(NGX_LOG_ERR, ctx->log, 0,
+                          "invalid nextUpdate time in certificate status");
+            goto error;
+        }
+
+    } else {
+        valid = NGX_MAX_TIME_T_VALUE;
     }
 
     OCSP_CERTID_free(id);

--
Maxim Dounin
http://nginx.org/

_______________________________________________
nginx-devel mailing list
nginx-devel at nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel