[PATCH] compositor: Fix buggy snprintf that sets module path

[Kristian Høgsberg]

On Wed, May 23, 2012 at 11:32:24PM -0700, Chad Versace wrote:
> If the MODULEDIR string contains '%', then
>     snprintf(path, sizeof(path), MODULEDIR "/%s", name);
> does not do what you want.

Heh, 'buggy' is a little harsh... the format string issue is only a
vulnerability if it's a (runtime) user-supplied string.  In this case,
MODULEDIR is specified at configure time and is a string literal in
the source.  So unless you say ./configure --libdir='lulz%n%n',
there's no problem.  Having said all that,
snprintf("%s/%s", MODULEDIR, name); is certainly better.

Kristian


> Fix this by replacing snprintf with stncpy followed by strncat.
> 
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>  src/compositor.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 3c1e058..e556e70 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2773,10 +2773,12 @@ load_module(const char *name, const char *entrypoint, void **handle)
>  	char path[PATH_MAX];
>  	void *module, *init;
>  
> -	if (name[0] != '/')
> -		snprintf(path, sizeof path, MODULEDIR "/%s", name);
> -	else
> +	if (name[0] != '/') {
> +		strncpy(path, MODULEDIR "/", sizeof(path) - 1);
> +		strncat(path, name, sizeof(path) - sizeof(MODULEDIR "/") - 1);
> +	} else {
>  		snprintf(path, sizeof path, "%s", name);
> +	}
>  
>  	module = dlopen(path, RTLD_LAZY);
>  	if (!module) {
> -- 
> 1.7.10.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel