Bug 2585 - smbpasswd segfaults because of bad *printf() calls
Summary: smbpasswd segfaults because of bad *printf() calls
Status: CLOSED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: Client Tools (show other bugs)
Version: 3.0.13
Hardware: x86 Linux
: P3 normal
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-04-08 15:32 UTC by Ulf Härnhammar
Modified: 2005-08-24 10:28 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ulf Härnhammar 2005-04-08 15:32:15 UTC
There are some bad *printf() calls in smbpasswd.c. They use user-defined data as
the format string instead of using it as a parameter to a fixed format string.
This causes segmentation faults with data such as "%n%n%n%n". I don't _think_
this is a security issue, because the data doesn't seem to be controllable from
remote machines. Nevertheless, a segfault is a segfault, so I thought this was
worth reporting.

Here is a stupid little patch:


--- source/utils/smbpasswd.c.old	2005-02-25 18:59:42.000000000 +0100
+++ source/utils/smbpasswd.c	2005-04-09 00:07:59.912213960 +0200
@@ -282,7 +282,7 @@ static BOOL password_change(const char *
 		ret = remote_password_change(remote_mach, username, 
 					     old_passwd, new_pw, err_str, sizeof(err_str));
 		if(*err_str)
-			fprintf(stderr, err_str);
+			fprintf(stderr, "%s", err_str);
 		return ret;
 	}
 	
@@ -290,9 +290,9 @@ static BOOL password_change(const char *
 				     err_str, sizeof(err_str), msg_str, sizeof(msg_str));
 
 	if(*msg_str)
-		printf(msg_str);
+		printf("%s", msg_str);
 	if(*err_str)
-		fprintf(stderr, err_str);
+		fprintf(stderr, "%s", err_str);
 
 	return ret;
 }


Here is a session capture of one of the possible ways to crash smbpasswd:


metaur@metaur:~/samba-3.0.13/source/bin$ ./smbpasswd -r %n%n%n%n
Old SMB password:
New SMB password:
Retype new SMB password:
Segmentation fault
metaur@metaur:~/samba-3.0.13/source/bin$ gdb ./smbpasswd
GNU gdb 6.3-debian
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-linux"...Using host libthread_db library
"/lib/tls/libthread_db.so.1".

(gdb) r -r %n%n%n%n
Starting program: /home/metaur/samba-3.0.13/source/bin/smbpasswd -r %n%n%n%n
Old SMB password:
New SMB password:
Retype new SMB password:

Program received signal SIGSEGV, Segmentation fault.
0x400c36f3 in vfprintf () from /lib/tls/libc.so.6
(gdb) bt
#0  0x400c36f3 in vfprintf () from /lib/tls/libc.so.6
#1  0x400c4f8e in vfprintf () from /lib/tls/libc.so.6
#2  0x400c0f7a in vfprintf () from /lib/tls/libc.so.6
#3  0x400c968f in fprintf () from /lib/tls/libc.so.6
#4  0x0806a610 in password_change (remote_mach=0xbffff094 "", username=0x8181ec0
"metaur", old_passwd=0x0, new_pw=0x0, local_flags=0)
    at utils/smbpasswd.c:285
#5  0x0806ad27 in process_nonroot (local_flags=64) at utils/smbpasswd.c:538
#6  0x0806ae8e in main (argc=0, argv=0x0) at utils/smbpasswd.c:595
(gdb) i r
eax            0x0      0
ecx            0xbffff094       -1073745772
edx            0x0      0
ebx            0x401acc60       1075498080
esp            0xbfffc374       0xbfffc374
ebp            0xbfffc970       0xbfffc970
esi            0x29     41
edi            0x400    1024
eip            0x400c36f3       0x400c36f3
eflags         0x10282  66178
cs             0x73     115
ss             0x7b     123
ds             0x7b     123
es             0x7b     123
fs             0x0      0
gs             0x33     51
(gdb) q
The program is running.  Exit anyway? (y or n) y
metaur@metaur:~/samba-3.0.13/source/bin$


// Ulf Härnhammar (metaur@telia.com)
Comment 1 Jeremy Allison 2005-04-08 15:55:34 UTC
Thanks - applied.
Jeremy.
Comment 2 Ulf Härnhammar 2005-04-08 16:22:17 UTC
Shouldn't the printf(msg_str); statement be fixed as well?
Comment 3 Gerald (Jerry) Carter (dead mail address) 2005-04-20 06:26:42 UTC
fixed in 3.0.15pre2
Comment 4 Gerald (Jerry) Carter (dead mail address) 2005-08-24 10:28:17 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.