[SSSD] [PATCH] Don't pass user input as a printf format string argument

Simo Sorce simo at redhat.com
Tue Jan 7 18:55:37 UTC 2014


A few notes:

1. style
although we allow things like 
if (condition)
    statement;
I do not think we accpet for or while loops w/o braces

2. the functions are incomplete and lack documentation that states so
in particular the only accepted format type is 's' (string).
I think we want a complete replacement for [a|sn]printf if you want to
put it in ding-libs.
If you do not want to make the general puprpose version then I propose
you contribute this only as a private function for sssd, but please add
documentation in the header file about what formatting is supported.

3. can you change from 'callback' to 'copy_fn' ?

On the code, looks mostly ok except the incompleteness of the printf
function, however:

On Tue, 2014-01-07 at 16:07 +0100, Stef Walter wrote:
> +static void
> +snprintf_callback (void *data,
> +                   const char *piece,
> +                   size_t length)
> +{
> +    struct sprintf_ctx *cx = data;
> +
> +    /* Don't copy if too much data */
> +    if (cx->length > cx->alloc)
> +        length = 0;
> +    else if (cx->length + length > cx->alloc)
> +        length = cx->alloc - cx->length;
> +    else
> +        length = length;

The last one branch is unnecessary and a bit useless :-)

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list