Bug 127973
Description
Josh Bressers
2004-07-15 21:03:03 UTC
This issue also affects FC2. RHEL is handled by bug 127974 Aug 04 1200UTC - removing embargo What is the proposed fix for this issue? There is none yet. Surely the perl-written extfs scripts need to be fixed and audited (I believe the shell scripts are ok), but my perl knowledge is very limited, so I'd appreciate if somebody who speaks perl could do that instead. The file name should of course *not* contain the backslashes. Created attachment 102849 [details]
Escapes dangerous chars
Escapes all characters not in a-z, A-Z, 0-9, _, /, ., - and +.
Should be tested on a system with dpkg installed to see if it actually works.
Files that call open or system and might be dangerous: a.in apt.in deba.in vulnerable (attaching patch) debd.in deb.in vulnerable (attaching improved patch) dpkg.in hp48.in patchfs.in not vulnerable afaict trpm ulha.in uzip.in not vulnerable uzoo.in Created attachment 102887 [details]
Escapes dangerous chars where necessary
Escape parameters for calls that open a shell.
Don't escape parameters for calls to open(FILEOUT, "> $destfile").
Created attachment 102888 [details]
Escapes dangerous chars where necessary in deba.in
Created attachment 102932 [details]
debd.in: Escapes parameters to system and open calls that spawn a shell
Created attachment 102933 [details]
dpkg.in: Escapes parameters to system and open calls that spawn a shell
Question: should the output of find be escape in this case: if ( open(PIPEIN, "find /var/cache/apt/archives -type f |") ) { ? Same for open STAT, "apt-cache dumpavail |" The above are no problem as there are no variables passed to the invoked shell. Too sum up my audit of the perl files in vfs/extfs with regard to the passing of unescaped parameters to system calls and open calls that spawn a shell I found the following: a.in vulnerable (attaching patch) apt.in vulnerable (attaching patch) deba.in vulnerable (patch attached) debd.in vulnerable (patch attached) deb.in vulnerable (patch attached) dpkg.in vulnerable (patch attached) mailfs.in not vulnerable afaict patchfs.in not vulnerable uzip.in not vulnerable (Files mentioned before but not here do not contain system calls, just the word system in the header ;) What about the '1;' at the end of a.in? Or is that just an indication of it's alphaness? Created attachment 102948 [details]
a.in: Escapes parameters to system and open calls that spawn a shell
Created attachment 102949 [details]
apt.in: Escapes parameters to system and open calls that spawn a shell
a.in ~ line 62 should read: ( $qdest, $qsrc ) = @ARGV; apt.in ~ line 282 should read: $qarchive =~ s%^CACHE/%/var/cache/apt/archives/%; ~ line 308 should read: $qarchive =~ s%^CACHE/%/var/cache/apt/archives/%; debd.in ~ lines 226/227 should read: $qfilename=~s!^CONTENTS!!; system("cat $qfilename > $qdestfile"); ~ line 257 should read: $qfilename=~s!^CONTENTS!!; deb.in ~ line 130 should read: $qfilename=~s!^DEBIAN/!!; ~ line 148 should read: $qfilename=~s!^CONTENTS/!!; Created attachment 102951 [details]
Comprehensive patch
Includes above fixes, code cleanup (quote instead of regexp).
Patch against 4.6.0 + jumbo.
Fixed the three omissions in a.in. Jakub, as you didn't answer my last mail to the mc-devel list yet I assume you have reviewed the whole patch and found it correct? Created attachment 102961 [details]
Comprehensive patch
Includes earlier fixes, code cleanup (quote instead of regexp).
Also added quotation for the three omissions in a.in.
Patch against 4.6.0 + jumbo.
Updates have been pushed yesterday. SUSE's security team has also audited the extfs shell scripts and came up with some extra fixes. Created attachment 104496 [details]
Comprehensive patch including fixes to shell scripts
Created attachment 104498 [details]
Cumulative patch (only shell script fixes)
Fixes to trpm are incomplete. Working on it. Created attachment 104795 [details]
extfs shell script parameter fixes
Only tested against CVS. If it doesn't apply just patch against CVS and diff
from there.
audio.in and hp48.in fixes should be ok. I think I fixed trpm correctly, but
please have a look to verify this.
Included to CVS. You should verify the code before applying. This is a work in progress. See mc-devel. I'll be looking at this patch next week. Andrew Somailov pointed out some ommissions in at least audio.in and hp48.in IIRC. Leonard, should I consider it fixed in upstream? I think the most of important extfs fixes are already applied and fixed in mc-4.6.1-0.9? Roland Illig recently posted some more fixes to the list (quoting related, but no real vulnerabilities) which he commited after I checked them. Mabye you should contact him to verify that he has double checked all the shell scripts in extfs. At least he hasn't updated the TODO, although he fixed four of the files mentioned in it. For Fedora Legacy project, this will also affect Fedora Core 1's mc, in addition to RHL 7.3 and 9. Cross reference Fedora Legacy Bug # 2009, at <http://bugzilla.fedora.us/show_bug.cgi?id=2009>. [Bulk move of FC2 bugs to Fedora Legacy. See <http://www.redhat.com/archives/fedora-announce-list/2005-April/msg00020.html>.] FL 2009 is now bug #152770. And this appears fixed in the released-to-testing-but-no-further bug #148865. (Scroll down a ways in the changelog for that package. *** This bug has been marked as a duplicate of 148865 *** This is not a duplicate of bug 148865. See that the CAN numbers are different. Jindrich, could you verify that the tarball you use(d) for FC2 contains all vfs fixes so we can close this bug ERRATA (I believe this to be the case)? Verified in bug 148865 that FC2 uses at least a 4.6.1-pre3 tarball. This issue no longer exists in that release. Closing ERRATA. Re: comment #33 -- yes, the CAN numbers are different, but that update includes this as well. (As I said, scroll down in the changelog for that package.) And that errata hasn't actually been released, unless you know something I don't. Still, leaving this closed as "errata", because presumably, when the package from bug #148865 is released, this will be covered, which is why I marked it as a duplicate in the first place. Dang you just beat me ;-) . As I was saying: Please use the latest testing release for FC 2, or go with the latest release for FC 3 on FC 2. I'm in touch with notting about this. After it's signed it can be safely moved to final. Since the update actually contains the pre3 tarball it should contain all necessary fixes for the noted CANs. I won't reopen this bug even if I think this bug is CLOSED/ERRATA prematurely because only the testing update is released. However you'll see the update is now in final from my update announce to fedora-announce-list after it's signed. Excellent, thanks. I'll mark bug #148865 CLOSED:ERRATA when I see it. After a little discussion on fedora-legacy-list we concluded that the best solution is to build the update within the fedora-legacy build system. Please read: http://www.redhat.com/archives/fedora-legacy-list/2005-April/msg00042.html Jesse, could you please announce the update here when it's built? Thanks, Jindrich The update for FC2's Midnight Commander was published by the Fedora Legacy Project as an ERRATA as part of Bug # 152889. Fedora Legacy's Errata notice is here: <http://tinyurl.com/db2h7>. Question: Since this bug seems to actually have been fully addressed in Bug #152889, with the Errata for FC2 actually issued as part of that bug, shouldn't this bug actually be reopened and closed as a duplicate of bug 152889, to point people in the direction of where work on this issue was truly completed? *** This bug has been marked as a duplicate of 152889 *** |