Bug 11771

Summary: Changing the password of another user as root causes 'passwd' to crash
Product: Busybox Reporter: Herve GOURMELON <hgourmelon>
Component: OtherAssignee: unassigned
Status: RESOLVED FIXED    
Severity: major CC: busybox-cvs
Priority: P5    
Version: 1.29.x   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Host: Target:
Build:

Description Herve GOURMELON 2019-04-10 15:23:33 UTC
I'm working on a Busybox applet compiled with shadow passwords enabled and Linux-PAM options also enabled.

For a given user 'johndoe', attempting to change the password as root causes the 'passwd' applet to crash, as shown below. Also affects 'adduser' when the administrator is asked to define the new password for the new user.


[root@C600-newKontron ~]# passwd johndoe 
Changing password for johndoe
New password: 
Retype password: 
*** Error in `passwd': free(): invalid pointer: 0x080d84dc ***
======= Backtrace: =========
/lib/libc.so.6(+0x6f620)[0xb75bf620]
/lib/libc.so.6(+0x75a22)[0xb75c5a22]
/lib/libc.so.6(+0x7620d)[0xb75c620d]
passwd[0x80527e6]
======= Memory map: ========
08048000-080da000 r-xp 00000000 b3:03 92         /bin/busybox
080da000-080db000 rw-p 00091000 b3:03 92         /bin/busybox
080db000-080fc000 rw-p 00000000 00:00 0          [heap]
b7506000-b751b000 r-xp 00000000 b3:03 65550      /lib/libgcc_s.so.1
b751b000-b751c000 rw-p 00014000 b3:03 65550      /lib/libgcc_s.so.1
b751f000-b752a000 r-xp 00000000 b3:03 65532      /lib/libnss_files-2.20.so
b752a000-b752b000 r--p 0000a000 b3:03 65532      /lib/libnss_files-2.20.so
b752b000-b752c000 rw-p 0000b000 b3:03 65532      /lib/libnss_files-2.20.so
b752c000-b752e000 rw-p 00000000 00:00 0 
b752e000-b7531000 r-xp 00000000 b3:03 65539      /lib/libdl-2.20.so
b7531000-b7532000 r--p 00002000 b3:03 65539      /lib/libdl-2.20.so
b7532000-b7533000 rw-p 00003000 b3:03 65539      /lib/libdl-2.20.so
b7533000-b754b000 r-xp 00000000 b3:03 65525      /lib/libpthread-2.20.so
b754b000-b754d000 r--p 00017000 b3:03 65525      /lib/libpthread-2.20.so
b754d000-b754e000 rw-p 00019000 b3:03 65525      /lib/libpthread-2.20.so
b754e000-b7550000 rw-p 00000000 00:00 0 
b7550000-b76f8000 r-xp 00000000 b3:03 65456      /lib/libc-2.20.so
b76f8000-b76fb000 r--p 001a7000 b3:03 65456      /lib/libc-2.20.so
b76fb000-b76fd000 rw-p 001aa000 b3:03 65456      /lib/libc-2.20.so
b76fd000-b7700000 rw-p 00000000 00:00 0 
b7700000-b7714000 r-xp 00000000 b3:03 65459      /lib/libresolv-2.20.so
b7714000-b7716000 r--p 00013000 b3:03 65459      /lib/libresolv-2.20.so
b7716000-b7717000 rw-p 00015000 b3:03 65459      /lib/libresolv-2.20.so
b7717000-b7719000 rw-p 00000000 00:00 0 
b7719000-b771b000 r-xp 00000000 b3:03 65464      /lib/libpam_misc.so.0.82.0
b771b000-b771c000 rw-p 00001000 b3:03 65464      /lib/libpam_misc.so.0.82.0
b771c000-b7726000 r-xp 00000000 b3:03 65549      /lib/libpam.so.0.83.1
b7726000-b7727000 rw-p 00009000 b3:03 65549      /lib/libpam.so.0.83.1
b7727000-b7742000 r-xp 00000000 b3:03 473        /usr/lib/libtirpc.so.3.0.0
b7742000-b7743000 rw-p 0001b000 b3:03 473        /usr/lib/libtirpc.so.3.0.0
b7744000-b7747000 rw-p 00000000 00:00 0 
b7747000-b7749000 r--p 00000000 00:00 0          [vvar]
b7749000-b774a000 r-xp 00000000 00:00 0          [vdso]
b774a000-b776c000 r-xp 00000000 b3:03 65455      /lib/ld-2.20.so
b776c000-b776d000 r--p 00021000 b3:03 65455      /lib/ld-2.20.so
b776d000-b776e000 rw-p 00022000 b3:03 65455      /lib/ld-2.20.so
bfe4a000-bfe6b000 rw-p 00000000 00:00 0          [stack]
Aborted
[root@C600-newKontron ~]#


The culprit seems to be an incorrectly initialized pointer in function  new_password() (loginutils/passwd.c) that we try to free when the function exits. 
Here's a patch that seemingly fixes the problem:

diff -urBN busybox-1.29.3/loginutils/passwd.c busybox-1.29.3-new/loginutils/passwd.c
--- busybox-1.29.3/loginutils/passwd.c  2018-07-02 13:23:06.000000000 +0200
+++ busybox-1.29.3-new/loginutils/passwd.c      2019-04-10 17:03:51.649975516 +0200
@@ -43,7 +43,7 @@
 static char* new_password(const struct passwd *pw, uid_t myuid, const char *algo)
 {
        char salt[MAX_PW_SALT_LEN];
-       char *orig = (char*)"";
+       char *orig = NULL;//(char*)"";
        char *newp = NULL;
        char *cp = NULL;
        char *ret = NULL; /* failure so far */
Comment 1 Herve GOURMELON 2019-04-10 16:04:52 UTC
Wrong patch, indented with spaces instead of tabulations. Here is the correct version:

diff -urBN busybox-1.29.3/loginutils/passwd.c busybox-1.29.3-new/loginutils/passwd.c
--- busybox-1.29.3/loginutils/passwd.c  2018-07-02 13:23:06.000000000 +0200
+++ busybox-1.29.3-new/loginutils/passwd.c      2019-04-10 17:03:51.649975516 +0200
@@ -43,7 +43,7 @@
 static char* new_password(const struct passwd *pw, uid_t myuid, const char *algo)
 {
 	char salt[MAX_PW_SALT_LEN];
-	char *orig = (char*)"";
+	char *orig = NULL;//(char*)"";
 	char *newp = NULL;
 	char *cp = NULL;
 	char *ret = NULL; /* failure so far */
Comment 2 Denys Vlasenko 2019-04-28 15:03:23 UTC
fixed in git:

commit ce51140664d82300d25b096b4a41f01fdfd766b3
Author: Einar Jón <tolvupostur@gmail.com>
Date:   Tue Jan 8 16:31:37 2019 +0100

    passwd: initialize pointers correctly