# HG changeset patch # User Sergey Kandaurov <pluk...@nginx.com> # Date 1665665717 -14400 # Thu Oct 13 16:55:17 2022 +0400 # Node ID b2eba2994ddcbf9084075f9ae32c3332a628ca7a # Parent 81b4326daac70d6de70abbc3fe36d4f6e3da54a2 SSL: improved validation of ssl_session_cache and ssl_ocsp_cache.
Now it properly detects invalid shared zone configuration with omitted size. Previously it used to read outside of the buffer boundary. Found with AddressSanitizer. diff --git a/src/http/modules/ngx_http_ssl_module.c b/src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.c +++ b/src/http/modules/ngx_http_ssl_module.c @@ -1039,10 +1039,10 @@ ngx_http_ssl_session_cache(ngx_conf_t *c { ngx_http_ssl_srv_conf_t *sscf = conf; - size_t len; + u_char *p; ngx_str_t *value, name, size; ngx_int_t n; - ngx_uint_t i, j; + ngx_uint_t i; value = cf->args->elts; @@ -1083,25 +1083,20 @@ ngx_http_ssl_session_cache(ngx_conf_t *c && ngx_strncmp(value[i].data, "shared:", sizeof("shared:") - 1) == 0) { - len = 0; + name.data = value[i].data + sizeof("shared:") - 1; + + p = (u_char *) ngx_strchr(name.data, ':'); - for (j = sizeof("shared:") - 1; j < value[i].len; j++) { - if (value[i].data[j] == ':') { - break; - } - - len++; + if (p == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "invalid zone size \"%V\"", &value[i]); + return NGX_CONF_ERROR; } - if (len == 0) { - goto invalid; - } + name.len = p - name.data; - name.len = len; - name.data = value[i].data + sizeof("shared:") - 1; - - size.len = value[i].len - j - 1; - size.data = name.data + len + 1; + size.data = p + 1; + size.len = value[i].data + value[i].len - size.data; n = ngx_parse_size(&size); @@ -1151,10 +1146,9 @@ ngx_http_ssl_ocsp_cache(ngx_conf_t *cf, { ngx_http_ssl_srv_conf_t *sscf = conf; - size_t len; - ngx_int_t n; - ngx_str_t *value, name, size; - ngx_uint_t j; + u_char *p; + ngx_int_t n; + ngx_str_t *value, name, size; if (sscf->ocsp_cache_zone != NGX_CONF_UNSET_PTR) { return "is duplicate"; @@ -1173,25 +1167,20 @@ ngx_http_ssl_ocsp_cache(ngx_conf_t *cf, goto invalid; } - len = 0; + name.data = value[1].data + sizeof("shared:") - 1; + + p = (u_char *) ngx_strchr(name.data, ':'); - for (j = sizeof("shared:") - 1; j < value[1].len; j++) { - if (value[1].data[j] == ':') { - break; - } - - len++; + if (p == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "invalid zone size \"%V\"", &value[1]); + return NGX_CONF_ERROR; } - if (len == 0) { - goto invalid; - } + name.len = p - name.data; - name.len = len; - name.data = value[1].data + sizeof("shared:") - 1; - - size.len = value[1].len - j - 1; - size.data = name.data + len + 1; + size.data = p + 1; + size.len = value[1].data + value[1].len - size.data; n = ngx_parse_size(&size); diff --git a/src/mail/ngx_mail_ssl_module.c b/src/mail/ngx_mail_ssl_module.c --- a/src/mail/ngx_mail_ssl_module.c +++ b/src/mail/ngx_mail_ssl_module.c @@ -628,10 +628,10 @@ ngx_mail_ssl_session_cache(ngx_conf_t *c { ngx_mail_ssl_conf_t *scf = conf; - size_t len; + u_char *p; ngx_str_t *value, name, size; ngx_int_t n; - ngx_uint_t i, j; + ngx_uint_t i; value = cf->args->elts; @@ -672,25 +672,20 @@ ngx_mail_ssl_session_cache(ngx_conf_t *c && ngx_strncmp(value[i].data, "shared:", sizeof("shared:") - 1) == 0) { - len = 0; + name.data = value[i].data + sizeof("shared:") - 1; + + p = (u_char *) ngx_strchr(name.data, ':'); - for (j = sizeof("shared:") - 1; j < value[i].len; j++) { - if (value[i].data[j] == ':') { - break; - } - - len++; + if (p == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "invalid zone size \"%V\"", &value[i]); + return NGX_CONF_ERROR; } - if (len == 0) { - goto invalid; - } + name.len = p - name.data; - name.len = len; - name.data = value[i].data + sizeof("shared:") - 1; - - size.len = value[i].len - j - 1; - size.data = name.data + len + 1; + size.data = p + 1; + size.len = value[i].data + value[i].len - size.data; n = ngx_parse_size(&size); diff --git a/src/stream/ngx_stream_ssl_module.c b/src/stream/ngx_stream_ssl_module.c --- a/src/stream/ngx_stream_ssl_module.c +++ b/src/stream/ngx_stream_ssl_module.c @@ -1019,10 +1019,10 @@ ngx_stream_ssl_session_cache(ngx_conf_t { ngx_stream_ssl_conf_t *scf = conf; - size_t len; + u_char *p; ngx_str_t *value, name, size; ngx_int_t n; - ngx_uint_t i, j; + ngx_uint_t i; value = cf->args->elts; @@ -1063,25 +1063,20 @@ ngx_stream_ssl_session_cache(ngx_conf_t && ngx_strncmp(value[i].data, "shared:", sizeof("shared:") - 1) == 0) { - len = 0; + name.data = value[i].data + sizeof("shared:") - 1; + + p = (u_char *) ngx_strchr(name.data, ':'); - for (j = sizeof("shared:") - 1; j < value[i].len; j++) { - if (value[i].data[j] == ':') { - break; - } - - len++; + if (p == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "invalid zone size \"%V\"", &value[i]); + return NGX_CONF_ERROR; } - if (len == 0) { - goto invalid; - } + name.len = p - name.data; - name.len = len; - name.data = value[i].data + sizeof("shared:") - 1; - - size.len = value[i].len - j - 1; - size.data = name.data + len + 1; + size.data = p + 1; + size.len = value[i].data + value[i].len - size.data; n = ngx_parse_size(&size); _______________________________________________ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org