[PATCH] SSL: improved validation of ssl_session_cache and ssl_ocsp_cache
Sergey Kandaurov
pluknet at nginx.com
Thu Oct 13 13:02:42 UTC 2022
# HG changeset patch
# User Sergey Kandaurov <pluknet at 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);
More information about the nginx-devel
mailing list