Package: fluxconf Version: 0.9.9.2-2 Severity: normal Tags: patch User: [EMAIL PROTECTED] Usertags: origin-ubuntu jaunty ubuntu-patch
Hi, In Ubuntu the compiler checks for more potential problems, notably not checking the return code of write(2) and using printf() style functions without a format string. The first of these is a problem if you get a short read, the second can be a security vulnerabilty if the attacker can control the string, and just cause crashes otherwise. I created the attached patch to try and rectify these issues. Thanks, James
--- fluxconf-0.9.9.2.orig/src/fluxmenu.c +++ fluxconf-0.9.9.2/src/fluxmenu.c @@ -268,7 +268,7 @@ file = fopen(initpath, "r"); if(file==NULL) { g_print(_("Can't open %s\n"), initpath); - g_print(_("Make sure you installed fluxbox with this user.\n")); + g_print("%s", _("Make sure you installed fluxbox with this user.\n")); exit(1); } while (fgets(buf, 200, file)) { @@ -286,7 +286,7 @@ file = fopen(initpath, "r"); if (file == NULL) { g_print("Can't open %s : %s\n", initpath,strerror(errno)); - g_print(ERRFLUXMENU); + g_print("%s", ERRFLUXMENU); exit(-1); } @@ -417,7 +417,7 @@ file = fopen(initpath, "r"); if(file==NULL) { g_print(_("Can't open %s\n"), initpath); - g_print(_("Make sure you installed fluxbox with this user.\n")); + g_print("%s", _("Make sure you installed fluxbox with this user.\n")); exit(1); } while (fgets(buf, 200, file)) { @@ -432,8 +432,8 @@ /* * try to make a backup */ - char tmp_buff[BUFSIZ+1],*ptr; - int source_fd, dest_fd, rdlen; + char tmp_buff[BUFSIZ+1],*ptr, *write_ptr; + int source_fd, dest_fd, rdlen, written = 0; memset(tmp_buff,0,BUFSIZ+1); @@ -447,8 +447,26 @@ if(dest_fd<0) close(dest_fd); if(source_fd<0) close(source_fd); } else { - while((rdlen=read(source_fd,tmp_buff,BUFSIZ))>0) - write(dest_fd,tmp_buff,rdlen); + while((rdlen=read(source_fd ,tmp_buff ,BUFSIZ))>0) { + write_ptr = tmp_buff; + do { + written = write(dest_fd, write_ptr, rdlen); + if (written == rdlen) + break; + if (written > 0) { + rdlen -= written; + write_ptr += written; + } + } while (written >= 0 || errno == EINTR); + if (written < 0) { + g_print ("Error while writing: %s", strerror(errno)); + exit (1); + } + } + if (rdlen < 0) { + g_print ("Error while reading: %s", strerror(errno)); + exit (1); + } close(source_fd); close(dest_fd); } @@ -456,7 +474,7 @@ file = fopen(initpath, "w"); if (file == NULL) { g_print(_("Can't open %s for writing\n"), initpath); - g_print(ERRFLUXMENU); + g_print("%s", ERRFLUXMENU); exit(1); } only in patch2: unchanged: --- fluxconf-0.9.9.2.orig/src/fluxconf.c +++ fluxconf-0.9.9.2/src/fluxconf.c @@ -166,8 +166,8 @@ /* * try to make a backup */ - char tmp_buff[BUFSIZ + 1], * ptr = NULL; - int source_fd, dest_fd, rdlen; + char tmp_buff[BUFSIZ + 1], * ptr = NULL, * write_ptr = NULL; + int source_fd, dest_fd, rdlen, written = 0; memset(tmp_buff, 0, BUFSIZ + 1); @@ -183,8 +183,26 @@ if (source_fd < 0) close(source_fd); } else { - while ((rdlen = read(source_fd, tmp_buff, BUFSIZ)) > 0) - write(dest_fd, tmp_buff, rdlen); + while ((rdlen = read(source_fd, tmp_buff, BUFSIZ)) > 0) { + write_ptr = tmp_buff; + do { + written = write(dest_fd, write_ptr, rdlen); + if (written == rdlen) + break; + if (written > 0) { + rdlen -= written; + write_ptr += written; + } + } while (written >= 0 || errno == EINTR); + if (written < 0) { + perror ("write"); + exit (1); + } + } + if (rdlen < 0) { + perror ("read"); + exit (1); + } close(source_fd); close(dest_fd); } @@ -241,7 +259,7 @@ #else smallwin = gtk_message_dialog_new(NULL, GTK_DIALOG_DESTROY_WITH_PARENT, GTK_MESSAGE_WARNING, GTK_BUTTONS_CLOSE, - _("Now you must restart fluxbox (using the root menu, not killing X). Don't forget to press \"Let fluxbox change the conf\" before exiting.")); + "%s", _("Now you must restart fluxbox (using the root menu, not killing X). Don't forget to press \"Let fluxbox change the conf\" before exiting.")); gtk_dialog_run(GTK_DIALOG(smallwin)); gtk_widget_destroy(smallwin); #endif only in patch2: unchanged: --- fluxconf-0.9.9.2.orig/src/fluxbare.c +++ fluxconf-0.9.9.2/src/fluxbare.c @@ -205,7 +205,7 @@ void checkptr(void *ptr) { if (ptr == NULL) { - g_print(_("No more memory :(\n")); + g_print("%s", _("No more memory :(\n")); exit(1); } } only in patch2: unchanged: --- fluxconf-0.9.9.2.orig/src/fluxkeys.c +++ fluxconf-0.9.9.2/src/fluxkeys.c @@ -15,6 +15,7 @@ * *************************************************************************** */ +#include <errno.h> #include <unistd.h> #include <stdlib.h> /* gentenv */ #include <stdio.h> /* gentenv */ @@ -77,7 +78,7 @@ file = fopen(initpath, "r"); if (file == NULL) { g_print(_("Can't open %s\n"), initpath); - g_print(_("Make sure you installed fluxbox with this user.\n")); + g_print("%s", _("Make sure you installed fluxbox with this user.\n")); exit(1); } while (fgets(buf, 200, file)) { @@ -92,7 +93,7 @@ file = fopen(initpath, "r"); if (file == NULL) { g_print(_("Can't open %s\n"), initpath); - g_print(ERRFLUXKEYS); + g_print("%s", ERRFLUXKEYS); exit(1); } @@ -291,7 +292,7 @@ file = fopen(initpath, "r"); if (file == NULL) { g_print(_("Can't open %s\n"), initpath); - g_print(_("Make sure you installed fluxbox with this user.\n")); + g_print("%s", _("Make sure you installed fluxbox with this user.\n")); exit(1); } while (fgets(buf, MAXPATHLEN, file)) { @@ -311,8 +312,8 @@ /* * FIXME: this is highly redundant */ - char tmp_buff[BUFSIZ + 1], *ptr; - int source_fd, dest_fd, rdlen; + char tmp_buff[BUFSIZ + 1], *ptr, *write_ptr; + int source_fd, dest_fd, rdlen, written = 0; memset(tmp_buff, 0, BUFSIZ + 1); @@ -328,16 +329,32 @@ if (source_fd < 0) close(source_fd); } else { - while ((rdlen = read(source_fd, tmp_buff, BUFSIZ)) > 0) - write(dest_fd, tmp_buff, rdlen); - close(source_fd); - close(dest_fd); + while((rdlen=read(source_fd ,tmp_buff ,BUFSIZ))>0) { + write_ptr = tmp_buff; + do { + written = write(dest_fd, write_ptr, rdlen); + if (written == rdlen) + break; + if (written > 0) { + rdlen -= written; + write_ptr += written; + } + } while (written >= 0 || errno == EINTR); + if (written < 0) { + g_print ("Error while writing: %s", strerror(errno)); + exit (1); + } + } + if (rdlen < 0) { + g_print ("Error while reading: %s", strerror(errno)); + exit (1); + } } } file = fopen(initpath, "w"); if (file == NULL) { g_print(_("Can't open %s for writing\n"), initpath); - g_print(ERRFLUXKEYS); + g_print("%s", ERRFLUXKEYS); exit(1); } @@ -347,9 +364,9 @@ * execact=(char*)malloc(EXELEN*sizeof(char)); */ #ifndef DEBUG - fprintf(file, _("!Generated by fluxkeys\n")); + fprintf(file, "%s", _("!Generated by fluxkeys\n")); #else - g_print(_("!Generated by fluxkeys\n")); + g_print("%s", _("!Generated by fluxkeys\n")); #endif for (x = 0; x < nblignes + 1; x++) { mod = MOD_NONE;