[BusyBox] patch to login, dmesg and obscure

Vladimir N. Oleynik dzo at simtreas.ru
Wed Jul 30 15:04:56 UTC 2003


Ronny,

> Looking at the issue in  libbb/obscure.c:password_check(),  I think there's 
> still an error in 1.00-pre2. The handpatch by Vladimir copies too much data, 
> the source string ain't that long which the SIZE argument to strncpy() say.

You tested your patch? :-0

see safe_strncpy realization:

         dst[size-1] = '\0'; 

         return strncpy(dst, src, size-1); 


look previous example again:

old="123"
safe_strncpy(old+3, old, 4) =>
old[3+4-1 = 6] = '\0';
old[3] = '1';
old[4] = '2';
old[5] = '3'

Absolutely correct.

>  	newmono = str_lower(bb_xstrdup(newval));
> -	lenwrap = strlen(old) * 2 + 1;
> -	wrapped = (char *) xmalloc(lenwrap);
> -	str_lower(strcpy(wrapped, old));
> +	lenwrap = strlen(old);
> +	wrapped = (char *) xmalloc(lenwrap * 2 + 1);
> +	strcpy(wrapped, newmono);

Why not called str_lower ?

> -	else {
> -		safe_strncpy(wrapped + lenwrap, wrapped, lenwrap + 1);
> -		if (strstr(wrapped, newmono))
> +	else if (strstr(safe_strncpy(wrapped+lenwrap, wrapped, lenwrap + 1), newmono)) {

safe_trncpy returning pointer to first argument, you compare &wrapped[lenwrap], but
require comparing new double spliting wrapped.

> -	bzero(newmono, strlen(newmono));
> -	bzero(wrapped, lenwrap);

Its memory inspect protection.


--w
vodz




More information about the busybox mailing list