Package: obexpushd
Version: 0.4-3
Severity: grave
Tags: security, patch
Justification: user security hole

Hi,

I forward this to Debian to take necessary steps.
I also attach the patch that fixes this issue. Note that this is exploitable 
with any mobile device by just naming a file to exploit this like:
;some command
or
`some command`

Steps taken to prevent this:
Use
 '%s'
instead of only
 %s
and replace all ' with _

The option to use scriptable output is not used by default.

This should take care of all such attacks. If you have further suggestion or 
still see problems, please contact me.

Thanks

HS

-- System Information:
Debian Release: testing/unstable
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.18.1
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
diff -Nurp obexpushd-0.4/src/obexpushd.c obexpushd-0.4_fixed/src/obexpushd.c
--- obexpushd-0.4/src/obexpushd.c	2006-10-25 10:30:30.000000000 +0200
+++ obexpushd-0.4_fixed/src/obexpushd.c	2006-11-06 12:38:28.343845590 +0100
@@ -263,37 +263,71 @@ int create_file (obex_t* handle, int mod
 	}
 }
 
+int put_close (obex_t* handle) {
+	struct file_data_t* data = OBEX_GetUserData(handle);
+	if (script) {
+		if (pclose(data->out) < 0)
+			return -errno;
+	} else {
+		if (fclose(data->out) == EOF)
+			return -errno;
+	}
+			return 0;
+}
+
 int put_open (obex_t* handle) {
+	int err = 0;
 	struct file_data_t* data = OBEX_GetUserData(handle);
+	
+	if (data->out)
+		err = put_close(handle);
+	if (err < 0)
+		return err;
 
 	if (script && strlen(script)) {
 		uint8_t* name = utf16to8(data->name);
+		char* type = (type? strdup(data->type): NULL);
 		char* cmd;
 
 		if (name) {
+			size_t i;
 			size_t size = strlen(script)+1;
-			size += 1+utf8len(name);
-			if (data->type)
-				size += 1+strlen(data->type);
+			size += 3+utf8len(name);
+			if (type)
+				size += 3+strlen(type);
 			cmd = malloc(size);
-			if (!cmd)
+			if (!cmd) {
+				free(name);
+				free(type);
 				return -ENOMEM;
+			}
 			memset(cmd,0,size);
-			sprintf(cmd, "%s %s %s",script, name, (data->type? data->type: ""));
+
+			/* clean name and type against attacks:
+			 * replace ' with _
+			 */
+			for (i=0; i < utf8len(name); ++i)
+				if (name[i] == '\'')
+					name[i] = '_';
+			if (!type) {
+				(void)snprintf(cmd, size, "%s '%s'", script, (char*)name);
+			} else {
+				for (i=0; i < strlen(type); ++i)
+					if (type[i] == '\'')
+						type[i] = '_';			
+				(void)snprintf(cmd, size, "%s '%s' '%s'", script, (char*)name,type);
+			}
 		} else {
 			cmd = strdup(script);
 		}
 
 		errno = 0;
 		data->out = popen(cmd,"w");
+		if (!data->out)
+			err = (errno? -errno: -ENOMEM);
 		free(cmd);
-		if (!data->out) {
-			if (errno == 0)
-				return -ENOMEM;
-			else
-				return -errno;
-		}
-		return 0;
+		free(name);
+		free(type);
 	} else {
 		int status = create_file(handle,O_WRONLY);
 
@@ -311,9 +345,8 @@ int put_open (obex_t* handle) {
 		if (data->type && strlen(data->type))
 			if (debug) printf("%u: file type: %s\n",data->id,data->type);
 		if (debug) printf("%u: total expected size: %zu byte(s)\n",data->id,data->length);
-	
-		return 0;
 	}
+	return err;
 }
 
 int put_write (obex_t* handle, const uint8_t* buf, int len) {
@@ -330,18 +363,6 @@ int put_write (obex_t* handle, const uin
 	return 0;
 }
 
-int put_close (obex_t* handle) {
-	struct file_data_t* data = OBEX_GetUserData(handle);
-	if (script) {
-		if (pclose(data->out) < 0)
-			return -errno;
-	} else {
-		if (fclose(data->out) == EOF)
-			return -errno;
-	}
-			return 0;
-}
-
 void obex_object_headers (obex_t* handle, obex_object_t* obj) {
 	uint8_t id = 0;
 	obex_headerdata_t value;

Reply via email to