hi, attached is a patch, that fixes 2 buffer overflows in app-arch/sharutils: the first one was discovered by ulf harnhammar; see: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=265904 for details. while looking through the code, i discovered one more overflow: try this: bash-2.05# unshar `perl -e 'print "A"x1500'`/tmp/testing unshar: /home/flo/AA[A*x]AAA/tmp/testingSegmentation fault (core dumped) attached patch fixes all two problems. best regards florian [rootshell]
Created attachment 40686 [details, diff] ... the patch
Mike you did one of the previous security fixes please do it again. Also this package lacks metadata.xml.
Comment on attachment 40686 [details, diff] ... the patch >diff -Naur ./sharutils-4.2.1/src/shar.c ./sharutils-4.2.1_new/src/shar.c >--- ./sharutils-4.2.1/src/shar.c 1999-09-10 21:20:41.000000000 +0200 >+++ ./sharutils-4.2.1_new/src/shar.c 2004-09-29 09:06:06.782294248 +0200 >@@ -1571,7 +1571,7 @@ > sprintf (command, "%s '%s'", CHARACTER_COUNT_COMMAND, local_name); > if (pfp = popen (command, "r"), pfp) > { >- char wc[BUFSIZ]; >+ char wc[BUFSIZ], tempform[50]; > const char *prefix = ""; > > if (did_md5) >@@ -1579,8 +1579,8 @@ > fputs (" else\n", output); > prefix = " "; > } >- >- fscanf (pfp, "%s", wc); >+ sprintf (tempform, "%%%ds", BUFSIZ - 1); >+ fscanf (pfp, tempform, wc); > fprintf (output, "\ > %s shar_count=\"`%s '%s'`\"\n\ > %s test %s -eq \"$shar_count\" ||\n\ >diff -Naur ./sharutils-4.2.1/src/unshar.c ./sharutils-4.2.1_new/src/unshar.c >--- ./sharutils-4.2.1/src/unshar.c 1995-11-21 17:22:14.000000000 +0100 >+++ ./sharutils-4.2.1_new/src/unshar.c 2004-09-29 09:06:12.216468128 +0200 >@@ -346,8 +346,8 @@ > { > size_t size_read; > FILE *file; >- char name_buffer[NAME_BUFFER_SIZE]; >- char copy_buffer[NAME_BUFFER_SIZE]; >+ char name_buffer[NAME_BUFFER_SIZE] = {'\0'}; >+ char copy_buffer[NAME_BUFFER_SIZE] = {'\0'}; > int optchar; > > program_name = argv[0]; >@@ -409,14 +409,15 @@ > if (optind < argc) > for (; optind < argc; optind++) > { >- if (argv[optind][0] == '/') >- stpcpy (name_buffer, argv[optind]); >- else >- { >- char *cp = stpcpy (name_buffer, current_directory); >- *cp++ = '/'; >- stpcpy (cp, argv[optind]); >- } >+ if (argv[optind][0] == '/') { >+ strncpy (name_buffer, argv[optind], sizeof(name_buffer)); >+ name_buffer[sizeof(name_buffer)-1] = '\0'; >+ } >+ else { >>> snprintf(name_buffer, sizeof(name_buffer),"%s/%s", current_directory, argv[optind]); >>> name_buffer[sizeof(name_buffer)-1] = '\0'; >+ } > if (file = fopen (name_buffer, "r"), !file) > error (EXIT_FAILURE, errno, name_buffer); > unarchive_shar_file (name_buffer, file);
please post an additional attachment instead of trying to edit it ... bugzilla mangles the patch horribly otherwise :/
Created attachment 40702 [details, diff] the revised (working !!!) patch
updated patch looks sane to me; added to 4.2.1-r10
Arches, please mark 4.2.1-r10 stable
-r10 wont build here.. it asks for app-arch/sharutils/files/sharutils-4.2.1-r6-gentoo.diff which doesnt exist.. should it point to sharutils-4.2.1-gentoo.patch or is it something else you forgot to commit ?
Note: There are far more than just 2 problem areas in sharutils. When I last reviewed it I saw some many problem areas that I just gave up and said this thing needs a complete rewrite. So if you guys are going to do a GLSA I _really_ suggest you look quite a bit closer at the code. Also due to the nature of sharutils it's probably best to work with upstream on this package.
app-arch/sharutils/files/sharutils-4.2.1-r6-gentoo.diff is deleted by portage emerge sync ... deleting app-arch/sharutils/files/sharutils-4.2.1-r6-gentoo.diff
vapier renamed the patch without fixing -r10.. I fixed it.. Holding off a little before marking it x86... Can anyone look at the other problems solar mentionned ?
Hmm dunno what to do about this package. It has no Gentoo maintainer, it has no clear upstream maintainer, and it's full of various flaws. Like solar said, it's not the first time we patch something in there (see bug 46998) and it needs a thorough audit. But unless someone volunteers to audit this package and coordinate fixes with upstream (see bug 46998 for pointers), it's probably better to patch vulnerabilities as they are found.
last upstream release was in 1999 to me that says this package is dead upstream (just like so many GNU utils) maybe someday, out of the blue, a maintainer will step up ... but for now, none exist
so what do we do now? test it and mark it stable or take it out of cvs?
why would we take it out of cvs ? marked amd64/arm/hppa/ia64/s390 stable
ppc stable now too
sparc stable then...
Stable on x86 too then...
Stable on alpha.
*** Bug 65846 has been marked as a duplicate of this bug. ***
What depends on sharutils? jaervosz asked me to review the GLSA draft, but frankly given solar's comment I would prefer pulling it out entirely. Is this feasible? If that's not a good option, for whatever reason, I'll go ahead and review, but someone (possi/probably me, if I have the time) really should conduct a more thorough security audit.
this package isnt going anywhere, stop thinking about removing it it's one of those old school standard utilities ... shar archives arent as popular anymore, but go back in time and they were pretty heavily used
GLSA 200410-01
Stable on mips.
stable on ppc64, thanks!
There is a 4.3.77 version out now, that fixes lots of buffer overflows. Perhaps you could package that? I don't think that version is 100% perfect either. I'll do a thorough audit of it, when I get the time. // Ulf Harnhammar Debian Security Audit Project
