[PATCH] Core: print "(null)" on NULL for "%s" in ngx_vslprintf

[Maxim Dounin]

Hello!

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

> # HG changeset patch
> # User Alexander A. Strelets <streletsaa at 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/