Hello!

On Tue, Mar 13, 2012 at 08:34:33PM +1300, Matthew Daley wrote:

> Hi,
>
> There is a small issue in the handling of the SSI command provided by
> the Perl module.
>
> It NULL-terminates the 'sub' parameter value before passing it to
> ngx_http_perl_eval_anon_sub, but this has the problem that if the
> parameter's length is the maximum allowable for SSI commands, it will
> write the NULL byte just past the end of the allocated buffer:
>
> ==14669== Invalid write of size 1
> ==14669== at 0x80A79E3: ngx_http_perl_ssi (ngx_http_perl_module.c:384)
> ==14669== by 0x80904CF: ngx_http_ssi_body_filter (ngx_http_ssi_filter_module.c:794)

[...]

> I have attached a patch which attempts to fix the problem by creating
> an appropriately-sized buffer and NULL-terminating a copy of the
> string to eval.

Nice catch, thanks. The patch is wrong though, see below.

[...]

> @@ -381,7 +382,6 @@
> ctx->ssi = ssi_ctx;
>
> handler = params[NGX_HTTP_PERL_SSI_SUB];
> - handler->data[handler->len] = '\0';
>
> {
>
> @@ -392,7 +392,7 @@
>
> /* the code is disabled to force the precompiled perl code using only */
>
> - ngx_http_perl_eval_anon_sub(aTHX_ handler, &sv);
> + ngx_http_perl_eval_anon_sub(aTHX_ r->pool, handler, &sv);

This code is indeed disabled, as stated in the comment above, it's
under #if 0.

The null-terminated string in handler is required for

sv = newSVpvn((char *) handler->data, handler->len);

below though, and your patch will break it.

Correct solution would be to reserve space for NUL character in
ssi module instead. Quick and dirty patch which should work:

--- a/src/http/modules/ngx_http_ssi_filter_module.c
+++ b/src/http/modules/ngx_http_ssi_filter_module.c
@@ -1204,7 +1204,7 @@ ngx_http_ssi_parse(ngx_http_request_t *r

if (ctx->value_buf == NULL) {
ctx->param->value.data = ngx_pnalloc(r->pool,
- ctx->value_len);
+ ctx->value_len + 1);
if (ctx->param->value.data == NULL) {
return NGX_ERROR;
}

Maxim Dounin

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