Bug 13496

Summary: bb_unsetenv maybe free a NULL memory
Product: Busybox Reporter: Rein <luoruncai>
Component: NetworkingAssignee: unassigned
Status: RESOLVED INVALID    
Severity: normal CC: busybox-cvs
Priority: P5    
Version: 1.32.x   
Target Milestone: ---   
Hardware: All   
OS: Linux   
Host: Target:
Build:

Description Rein 2021-01-29 04:34:37 UTC
In the version 1.32.1,in file: busybox-1.32.1/libbb/xfuncs_printf.c

fuction: void FAST_FUNC bb_unsetenv(const char *var):

the origin code is bellow:
void FAST_FUNC bb_unsetenv(const char *var)
{
	char onstack[128 - 16]; /* smaller stack setup code on x86 */
	char *tp;

	tp = strchr(var, '=');
	if (tp) {
		/* In case var was putenv'ed, we can't replace '='
		 * with NUL and unsetenv(var) - it won't work,
		 * env is modified by the replacement, unsetenv
		 * sees "VAR" instead of "VAR=VAL" and does not remove it!
		 * Horror :(
		 */
		unsigned sz = tp - var;
		if (sz < sizeof(onstack)) {
			((char*)mempcpy(onstack, var, sz))[0] = '\0';
			tp = NULL;
			var = onstack;
		} else {
			/* unlikely: very long var name */
			var = tp = xstrndup(var, sz);
		}
	}
	unsetenv(var);
        free(tp);  // --- tp maybe a NULL when sz < sizeof(onstack)
}


so, my idea is :
if (tp != NULL) free(tp);


Thanks.
Comment 1 Rein 2021-01-29 05:09:04 UTC
busybox-1.32.1/libbb/xfuncs_printf.c:

void FAST_FUNC bb_unsetenv(const char *var)
{
	char onstack[128 - 16]; /* smaller stack setup code on x86 */
	char *tp;

	tp = strchr(var, '=');
	if (tp) {
		/* In case var was putenv'ed, we can't replace '='
		 * with NUL and unsetenv(var) - it won't work,
		 * env is modified by the replacement, unsetenv
		 * sees "VAR" instead of "VAR=VAL" and does not remove it!
		 * Horror :(
		 */
		unsigned sz = tp - var;
		if (sz < sizeof(onstack)) {
			((char*)mempcpy(onstack, var, sz))[0] = '\0';
                        //donot use mempcpy,this maybe abort the process
                        //I chage to snprintf(onstack, sz + 1, "%s", var); fix it
			tp = NULL;
			var = onstack;
		} else {
			/* unlikely: very long var name */
			var = tp = xstrndup(var, sz);
		}
	}
	unsetenv(var);
        free(tp);  // --- tp maybe a NULL when sz < sizeof(onstack)
}
Comment 2 Bernhard Reutner-Fischer 2021-02-06 10:43:53 UTC
ISO-IEC 9899, 7.20.3.2 free()

2 ... If ptr is a null pointer, no action occurs.

It is perfectly fine to free(NULL).