[PATCH] Always pass a format string to syslog()

[GoogleCodeExporter]

Our logging code looks like this currently:
    vsnprintf(&buf[offset], STRSIZE - offset, logstring, argp);
    syslog(LOG_CRIT, buf);
Passing 'buf' to syslog() like this is a problem, because we cannot guarantee 
that 'buf' does not contain further format specifiers such as %s.

Eg, consider what would happen if this line in messagehandler.c
    Log_info_client(client, "Server reject reason: %s", reason);
was called with reason being "foo %s".

Fix the problem by changing the syslog() calls to
   syslog(LOG_*, "%s", buf)

Note that the attached diff goes on top of the one I sent in issue 16.

Original issue reported on code.google.com by tilm...@googlemail.com on 22 Jan 2011 at 10:26

Attachments:

• syslog_literal.diff

[GoogleCodeExporter]

I see the problem, but just to make it clear: 
With the current code there is a risk that syslog() will try to dereference a 
non-existent pointer, probably resulting in a segfault.
With your fix, it will literally print "foo %s" in the log.
Correct?

Original comment by fatbob.s...@gmail.com on 23 Jan 2011 at 11:13

• Changed state: Accepted

[GoogleCodeExporter]

Original comment by fatbob.s...@gmail.com on 23 Jan 2011 at 11:13

[GoogleCodeExporter]

Yes. It might result in a segfault. I think it can be used to execute code even.

Original comment by tilm...@googlemail.com on 23 Jan 2011 at 11:46

[GoogleCodeExporter]

Committed in r154.
Thanks for your work!

Original comment by fatbob.s...@gmail.com on 23 Jan 2011 at 12:08

• Changed state: Fixed