[PATCH] Do not leave random talloc magic in free()'ed memory, fix abort message

[Andrew Bartlett]

On Wed, 2018-01-17 at 08:30 +0100, Volker Lendecke via samba-technical
wrote:
> On Wed, Jan 17, 2018 at 07:23:59AM +1300, Andrew Bartlett via samba-technical wrote:
> > Gary started to filter some untrusted strings when doing the auth audit
> > work (the *new* messages get the usernames etc encoded if not ASCII)
> > but what I'm saying is that every other DEBUG(0, ...), DEBUG(1, ...) et
> > al that uses %s on untrusted user-supplied data is the same threat.
> 
> You're saying that printf("%s",untrusted); is unsafe? What's the
> alternative? Along that line: How do we untaint data for Coverity?

In the context of DEBUG(), the attack is described here:

https://www.owasp.org/index.php/Log_Injection

For that reason, we took care to write and use log_escape() in the auth
audit code.  It is declared here:

lib/util/util_str_escape.c:char *log_escape(TALLOC_CTX *frame, const
char *in)

The list of 'bad' characters could potentially be extended.

I don't know and didn't assert that printf("%s", untrusted) is itself
unsafe, beyond the above I would see the main risk as being a SIGSEGV
if the attacker can control the NUL termination.

Thanks,

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba