Welcome! Log In Create A New Profile

Advanced

[nginx] Negative size buffers detection.

Maxim Dounin
November 26, 2018 11:44AM
details: https://hg.nginx.org/nginx/rev/a7ff19afbb14
branches:
changeset: 7401:a7ff19afbb14
user: Maxim Dounin <mdounin@mdounin.ru>
date: Mon Nov 26 18:29:56 2018 +0300
description:
Negative size buffers detection.

In the past, there were several security issues which resulted in
worker process memory disclosure due to buffers with negative size.
It looks reasonable to check for such buffers in various places,
much like we already check for zero size buffers.

While here, removed "#if 1 / #endif" around zero size buffer checks.
It looks highly unlikely that we'll disable these checks anytime soon.

diffstat:

src/core/ngx_output_chain.c | 64 +++++++++++++++++++++++++++-
src/http/ngx_http_write_filter_module.c | 40 ++++++++++++++++-
src/stream/ngx_stream_write_filter_module.c | 40 ++++++++++++++++-
3 files changed, 132 insertions(+), 12 deletions(-)

diffs (244 lines):

diff --git a/src/core/ngx_output_chain.c b/src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -126,6 +126,26 @@ ngx_output_chain(ngx_output_chain_ctx_t
continue;
}

+ if (bsize < 0) {
+
+ ngx_log_error(NGX_LOG_ALERT, ctx->pool->log, 0,
+ "negative size buf in output "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ ctx->in->buf->temporary,
+ ctx->in->buf->recycled,
+ ctx->in->buf->in_file,
+ ctx->in->buf->start,
+ ctx->in->buf->pos,
+ ctx->in->buf->last,
+ ctx->in->buf->file,
+ ctx->in->buf->file_pos,
+ ctx->in->buf->file_last);
+
+ ngx_debug_point();
+
+ return NGX_ERROR;
+ }
+
if (ngx_output_chain_as_is(ctx, ctx->in->buf)) {

/* move the chain link to the output chain */
@@ -665,7 +685,6 @@ ngx_chain_writer(void *data, ngx_chain_t

for (size = 0; in; in = in->next) {

-#if 1
if (ngx_buf_size(in->buf) == 0 && !ngx_buf_special(in->buf)) {

ngx_log_error(NGX_LOG_ALERT, ctx->pool->log, 0,
@@ -685,7 +704,26 @@ ngx_chain_writer(void *data, ngx_chain_t

continue;
}
-#endif
+
+ if (ngx_buf_size(in->buf) < 0) {
+
+ ngx_log_error(NGX_LOG_ALERT, ctx->pool->log, 0,
+ "negative size buf in chain writer "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ in->buf->temporary,
+ in->buf->recycled,
+ in->buf->in_file,
+ in->buf->start,
+ in->buf->pos,
+ in->buf->last,
+ in->buf->file,
+ in->buf->file_pos,
+ in->buf->file_last);
+
+ ngx_debug_point();
+
+ return NGX_ERROR;
+ }

size += ngx_buf_size(in->buf);

@@ -709,7 +747,6 @@ ngx_chain_writer(void *data, ngx_chain_t

for (cl = ctx->out; cl; cl = cl->next) {

-#if 1
if (ngx_buf_size(cl->buf) == 0 && !ngx_buf_special(cl->buf)) {

ngx_log_error(NGX_LOG_ALERT, ctx->pool->log, 0,
@@ -729,7 +766,26 @@ ngx_chain_writer(void *data, ngx_chain_t

continue;
}
-#endif
+
+ if (ngx_buf_size(cl->buf) < 0) {
+
+ ngx_log_error(NGX_LOG_ALERT, ctx->pool->log, 0,
+ "negative size buf in chain writer "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ cl->buf->temporary,
+ cl->buf->recycled,
+ cl->buf->in_file,
+ cl->buf->start,
+ cl->buf->pos,
+ cl->buf->last,
+ cl->buf->file,
+ cl->buf->file_pos,
+ cl->buf->file_last);
+
+ ngx_debug_point();
+
+ return NGX_ERROR;
+ }

size += ngx_buf_size(cl->buf);
}
diff --git a/src/http/ngx_http_write_filter_module.c b/src/http/ngx_http_write_filter_module.c
--- a/src/http/ngx_http_write_filter_module.c
+++ b/src/http/ngx_http_write_filter_module.c
@@ -80,7 +80,6 @@ ngx_http_write_filter(ngx_http_request_t
cl->buf->file_pos,
cl->buf->file_last - cl->buf->file_pos);

-#if 1
if (ngx_buf_size(cl->buf) == 0 && !ngx_buf_special(cl->buf)) {
ngx_log_error(NGX_LOG_ALERT, c->log, 0,
"zero size buf in writer "
@@ -98,7 +97,24 @@ ngx_http_write_filter(ngx_http_request_t
ngx_debug_point();
return NGX_ERROR;
}
-#endif
+
+ if (ngx_buf_size(cl->buf) < 0) {
+ ngx_log_error(NGX_LOG_ALERT, c->log, 0,
+ "negative size buf in writer "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ cl->buf->temporary,
+ cl->buf->recycled,
+ cl->buf->in_file,
+ cl->buf->start,
+ cl->buf->pos,
+ cl->buf->last,
+ cl->buf->file,
+ cl->buf->file_pos,
+ cl->buf->file_last);
+
+ ngx_debug_point();
+ return NGX_ERROR;
+ }

size += ngx_buf_size(cl->buf);

@@ -136,7 +152,6 @@ ngx_http_write_filter(ngx_http_request_t
cl->buf->file_pos,
cl->buf->file_last - cl->buf->file_pos);

-#if 1
if (ngx_buf_size(cl->buf) == 0 && !ngx_buf_special(cl->buf)) {
ngx_log_error(NGX_LOG_ALERT, c->log, 0,
"zero size buf in writer "
@@ -154,7 +169,24 @@ ngx_http_write_filter(ngx_http_request_t
ngx_debug_point();
return NGX_ERROR;
}
-#endif
+
+ if (ngx_buf_size(cl->buf) < 0) {
+ ngx_log_error(NGX_LOG_ALERT, c->log, 0,
+ "negative size buf in writer "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ cl->buf->temporary,
+ cl->buf->recycled,
+ cl->buf->in_file,
+ cl->buf->start,
+ cl->buf->pos,
+ cl->buf->last,
+ cl->buf->file,
+ cl->buf->file_pos,
+ cl->buf->file_last);
+
+ ngx_debug_point();
+ return NGX_ERROR;
+ }

size += ngx_buf_size(cl->buf);

diff --git a/src/stream/ngx_stream_write_filter_module.c b/src/stream/ngx_stream_write_filter_module.c
--- a/src/stream/ngx_stream_write_filter_module.c
+++ b/src/stream/ngx_stream_write_filter_module.c
@@ -104,7 +104,6 @@ ngx_stream_write_filter(ngx_stream_sessi
cl->buf->file_pos,
cl->buf->file_last - cl->buf->file_pos);

-#if 1
if (ngx_buf_size(cl->buf) == 0 && !ngx_buf_special(cl->buf)) {
ngx_log_error(NGX_LOG_ALERT, c->log, 0,
"zero size buf in writer "
@@ -122,7 +121,24 @@ ngx_stream_write_filter(ngx_stream_sessi
ngx_debug_point();
return NGX_ERROR;
}
-#endif
+
+ if (ngx_buf_size(cl->buf) < 0) {
+ ngx_log_error(NGX_LOG_ALERT, c->log, 0,
+ "negative size buf in writer "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ cl->buf->temporary,
+ cl->buf->recycled,
+ cl->buf->in_file,
+ cl->buf->start,
+ cl->buf->pos,
+ cl->buf->last,
+ cl->buf->file,
+ cl->buf->file_pos,
+ cl->buf->file_last);
+
+ ngx_debug_point();
+ return NGX_ERROR;
+ }

size += ngx_buf_size(cl->buf);

@@ -160,7 +176,6 @@ ngx_stream_write_filter(ngx_stream_sessi
cl->buf->file_pos,
cl->buf->file_last - cl->buf->file_pos);

-#if 1
if (ngx_buf_size(cl->buf) == 0 && !ngx_buf_special(cl->buf)) {
ngx_log_error(NGX_LOG_ALERT, c->log, 0,
"zero size buf in writer "
@@ -178,7 +193,24 @@ ngx_stream_write_filter(ngx_stream_sessi
ngx_debug_point();
return NGX_ERROR;
}
-#endif
+
+ if (ngx_buf_size(cl->buf) < 0) {
+ ngx_log_error(NGX_LOG_ALERT, c->log, 0,
+ "negative size buf in writer "
+ "t:%d r:%d f:%d %p %p-%p %p %O-%O",
+ cl->buf->temporary,
+ cl->buf->recycled,
+ cl->buf->in_file,
+ cl->buf->start,
+ cl->buf->pos,
+ cl->buf->last,
+ cl->buf->file,
+ cl->buf->file_pos,
+ cl->buf->file_last);
+
+ ngx_debug_point();
+ return NGX_ERROR;
+ }

size += ngx_buf_size(cl->buf);

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[nginx] Negative size buffers detection.

Maxim Dounin 340 November 26, 2018 11:44AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 319
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready