Found vulnerability in ngx_http_push_stream_complex_value

[StepanGulyaev]

Hello! I was analyzing your module with Svace SAST tool and found vulnerability in ngx_http_push_stream_complex_value. The problem is ngx_http_complex_value function return value not being checked:

nginx-push-stream-module/src/ngx_http_push_stream_module_utils.c

Line 2122 in b1c1a08

ngx_http_complex_value(r, val, value);

But it should because of this part in ngx_http_complex_value:

https://github.com/nginx/nginx/blob/d31305653701bd99e8e5e6aa48094599a08f9f12/src/http/ngx_http_script.c#L79-L90

If allocation with ngx_palloc fails and returns NULL, value->data will be NULL but value->len won`t be zero. So when we come back from ngx_http_complex_value and step into ngx_http_push_stream_unescape_uri we will see that this part of code relies only on value->len not being zero:

nginx-push-stream-module/src/ngx_http_push_stream_module_utils.c

Lines 2132 to 2140 in b1c1a08

	if (value->len) {
	dst = value->data;
	src = value->data;
	ngx_unescape_uri(&dst, &src, value->len, NGX_UNESCAPE_URI);
	if (dst < src) {
	*dst = '\0';
	value->len = dst - value->data;
	}
	}

But as I mentioned above there can be a situation where value->len is not zero but value->data is NULL. Dst and src values both became NULL and go as arguments into ngx_unescape_uri function which neither checks them being NULL.
https://github.com/nginx/nginx/blob/d31305653701bd99e8e5e6aa48094599a08f9f12/src/core/ngx_string.c#L1676-L1694

So there will be a crash right here, where "s" is dereferenced.

I've changed ngx_http_push_stream_complex_value in my pull request and made it return either NGX_ERROR or NGX_OK and have changed all places where it is being called also added logging where necessary.

It compiles well, haven't seen any trouble, and it passes 399 tests. It has two failures, one of them is "Measure Memory should check subscribers system size" (./spec/mix/measure_memory_spec.rb:105) but it happens also with current github version.

The only thing I found uniquely troublesome with my version is "Publisher Messages should expose message size through message template" test failed (./spec/publisher/publish_messages_spec.rb:333). It happens only when I test with my commits but I`m lacking experience with this module to trace it properly so my pull request should be modified a bit I guess. But I have no idea where.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

[sunnychun]

您的信已收到，我将尽快给您答复

[wandenberg]

@StepanGulyaev thanks for the evaluation and contribution. Please, allow me a bit longer to review and merge due to the complexity of the change.