This is really just papering over the issue, as it doesn't actually
validate the contents of 'me', which could still potentially contain
bad data.  But as this doesn't actually make anything *worse* and
fixes your case, applied as r967.

The "right" way to fix this is a lot more work involves abstracting
all the config stuff away behind a config validation layer.

I don't remember if we support taint mode in general or not.

-R


Jeff King wrote:
> 
> The code feeds the results of $session->config('me') to
> sprintf as part of the format string. In practice, this is
> probably not a problem since hostnames don't contain percent
> signs. However, it triggers a taint warning in perl 5.10,
> making cram-md5 auth unusable.
> 
> This patch rewrites the sprintf to insert the 'me' value
> using a %s format specifier.
> 
> ---
> I don't know the usual practice for submitting patches to qpsmtpd, so
> please let me know if I should be doing something differently.
> 
> I was a little confused by the test infrastructure, so no test, but
> hopefully this change is Obviously Correct. I can trigger it on my
> Debian testing and unstable boxen with just this plugin:
> 
>   sub hook_auth_cram_md5 {
>       return (DECLINED);
>   }
> 
> which generates this in the log:
> 
>   1732 XX: Insecure dependency in sprintf while running with -T switch at
>        lib/Qpsmtpd/Auth.pm line 63, <STDIN> line 3.
>   ./qpsmtpd[1732]: command 'auth' failed unexpectedly (Bad file descriptor)
> 
>  lib/Qpsmtpd/Auth.pm |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/Qpsmtpd/Auth.pm b/lib/Qpsmtpd/Auth.pm
> index 6e9a2a5..635491a 100644
> --- a/lib/Qpsmtpd/Auth.pm
> +++ b/lib/Qpsmtpd/Auth.pm
> @@ -60,8 +60,8 @@ sub SASL {
>          # rand() is not cryptographic, but we only need to generate a 
> globally
>          # unique number.  The rand() is there in case the user logs in more 
> than
>          # once in the same second, of if the clock is skewed.
> -        $ticket = sprintf( "<%x.%x\@" . $session->config("me") . ">",
> -            rand(1000000), time() );
> +        $ticket = sprintf( '<%x...@%s>',
> +            rand(1000000), time(), $session->config("me") );
>  
>          # We send the ticket encoded in Base64
>          $session->respond( 334, encode_base64( $ticket, "" ) );
> -- 
> 1.6.1.62.g8269b

Reply via email to