Hello!

On Thu, Apr 04, 2019 at 07:56:23PM +0300, Александр А. Стрелец wrote:

> # HG changeset patch
> # User Alexander A. Strelets <streletsaa@gmail.com>
> # Date 1554396620 -10800
> # Thu Apr 04 19:50:20 2019 +0300
> # Node ID cf212fc46cb7025b33dfb7313b5191eb09330446
> # Parent 7da71a7b141abf417d334dfaca8a13f01c9fe3f5
> Core: print "(null)" on NULL for "%s" in ngx_vslprintf
>
> When NULL is passes with arguments for "%s" format specifier to ngx_vslprintf (or other ngx_printf-like derivative), ngx_vslprintf really tries to access 0 address which triggers SIGSEGV, while conventional printf just literally prints "(null)" without an error. This is a reach source of hidden bugs in numerous calls to ngx_log_error, ngx_log_debug, etc. which are normally not called and not covered with tests. This case may be hit, for example, with ngx_log_error(NGX_LOG_CRIT, file->log, ngx_errno, "pread() \"%s\" failed", file->name.data); in nginx/src/os/unix/ngx_files.c, lines 40-46, in case file->name.data was left initially assigned with NULL (i.e. the user gets SIGSEGV instead of an [error]-record in the log).

Thank you for your patch.

Unfortunately, such approach does not help to identify such hidden
bugs you've mentioned, but rather further hides them. As such, I
would rather prefer to keep the code as is - intolerant to
incorrect pointers. This makes finding bugs easier. If you think
you know any bugs like this in nginx code - please report.

Note well that "conventional printf" behaviour is undefined when
NULL is used as an argument to %s. It may print "(null)", or
erase your hard drive, or make demons fly out of your nose.
You probably don't want to rely on "(null)" - unless you are
actually trying to summon nasal demons.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel