I also found this vulnerability this night. I also made a patch that I was intended to include in next debian version:
--- obexpushd-0.4.orig/src/obexpushd.c
+++ obexpushd-0.4/src/obexpushd.c
@@ -268,30 +268,49 @@
if (script && strlen(script)) {
uint8_t* name = utf16to8(data->name);
- char* cmd;
+ char *args[4] = {script, NULL, NULL, NULL};
+ pid_t child;
+ int fds[2];
if (name) {
- size_t size = strlen(script)+1;
- size += 1+utf8len(name);
+ args[1] = (char *)name;
if (data->type)
- size += 1+strlen(data->type);
- cmd = malloc(size);
- if (!cmd)
- return -ENOMEM;
- memset(cmd,0,size);
- sprintf(cmd, "%s %s %s",script, name, (data->type?
data->type: ""));
- } else {
- cmd = strdup(script);
+ args[2] = data->type;
}
- errno = 0;
- data->out = popen(cmd,"w");
- free(cmd);
+
+ if (pipe(fds) == -1) {
+ perror("pipe");
+ return -1;
+ }
+
+ child = fork();
+
+ switch (child) {
+ case -1:
+ perror("fork");
+ close(fds[0]);
+ close(fds[1]);
+ return -1;
+ case 0:
+ close(fds[0]);
+ break;
+ default:
+ close(fds[1]);
+ if (fds[0] != 0) {
+ dup2(0, fds[0]);
+ close(fds[0]);
+ }
+ execvp(script, args);
+ perror("execvp");
+ exit(EXIT_FAILURE);
+ }
+
+ data->out = fdopen(fds[1], "w");
if (!data->out) {
- if (errno == 0)
- return -ENOMEM;
- else
- return -errno;
+ close(fds[1]);
+ perror("fdopen(fds[1])");
+ return -1;
}
return 0;
} else {
I think that is better solution, it allows to have ' in file names. Also how
about
clients that send filenames with / in them? Does obexpushd handle them. It
should
use only last component in that case IMHO.
And it will be good to add warning to manual page, that called scripts
should check their parameters.
6 листопада 2006 о 12:44 +0100 Hendrik Sattler написав(-ла):
> 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;
--
Eugeniy Meshcheryakov
signature.asc
Description: Digital signature
