dev-libs/libcdio-0.78.2 provides iso-info and cd-info both exhibit this vulnerability. A properly formatted iso image can produce a coredump and may allow for malicious code injection. Reproducible: Always Steps to Reproduce: 1. mkdir -p tmp/dir1 2. echo file_with_really_really_long_silly_name_to_test_iso_info_buffer 3. mkisofs -J -R -volid My_Image -o test.iso tmp 4. iso-info -l test.iso Actual Results: iso-info lists files and directories up to the ver long file name at which point it segfaults. Expected Results: a listing of the directories in src/iso-info.c, in function "print_iso9660_recurse" there is a block of code that begins: _CDIO_LIST_FOREACH (entnode, entlist) { iso9660_stat_t *p_statbuf = _cdio_list_node_data (entnode); char *psz_iso_name = p_statbuf->filename; char _fullname[4096] = { 0, }; char translated_name[MAX_ISONAME+1]; if (yep != p_statbuf->rr.b3_rock || 1 == opts.no_rock_ridge) { iso9660_name_translate_ext(psz_iso_name, translated_name, i_joliet_level); snprintf (_fullname, sizeof (_fullname), "%s%s", psz_path, translated_name); } else { snprintf (_fullname, sizeof (_fullname), "%s%s", psz_path, psz_iso_name); I believe either the test "yep != p_statbuf->rr.b3_rock" is incorrect, or possibly the code in lib/iso9660 that sets "b3_rock". The field "b3_rock" is an enumeration with the values "nope", "yep", "dunno". The test would be valid if "b3_rock" was a boolean, but the additional "dunno" state causes a problem. When the iso is created with the options given above, the value of "b3_rock" is "dunno" (integer 2). This causes the code to enter the "iso9660_name_translate_ext" which writes beyond the end of "translated_name" and corrupts the stack. I have also submitted this upstream as bug #21910
The upstream bug report is marked private, so I restricted this bug for now. Please be aware that this vulnerability cannot be considered confidential anymore, as it was public in this bugtracker before. Diego, please advise.
Sorry about that, I didn't see an obvious way to enter a private bug. It's currently fixed in upstream CVS (iso-info.c:1.36, cd-info.c:1.150). I'm attaching a patch that fixes the current ebuild
To enter restricted bugs, just click the "Only users in all of the selected groups can view this bug: Gentoo Security" checkbox in the expert bug filing page.
I don't really know what to advise on such short notice as I didn't even know of this before. I can tell there are quite some changes in libcdio CVS, good ones mostly but I wouldn't fast track to stable a new release of it right now (and I say this because I did make some of the changes ;)), so I'd suppose extracting the commit patch would probably be the best thing.
Created attachment 139644 [details, diff] libcdio-info-buffer.patch Upstream committed patch
Diego, sounds good -- find the patch attached. As it is already in the upstream CVS, please prepare an ebuild.
libcdio-0.78.2-r2 added to portage.. although I'm not the libcdio ebuild maintainer since almost an year ;) Please when you add arches add also mips so that they can keyword a recent version, thanks.
Arch Security Liaisons, please test and mark stable dev-libs/libcdio-0.78.2-r2. Target keywords : "alpha amd64 arm hppa ia64 ppc ppc64 sh sparc x86" CC'ing current Liaisons: alpha : ferdy amd64 : welp hppa : jer ppc : dertobi123 ppc64 : corsair sparc : fmccor x86 : opfer
(In reply to comment #8) > Arch Security Liaisons, please test and mark stable dev-libs/libcdio-0.78.2-r2. > Target keywords : "alpha amd64 arm hppa ia64 ppc ppc64 sh sparc x86" Works on x86, but I am not able to commit to other directories than profiles at the moment...crappy lappy. Anyway, anyone is free to fix it for x86.
Stable for HPPA and x86 (comment #9).
adding ranger because my machine is down. ranger: comment #8
ppc64 done
amd64 done.
Rocky mailed this on libcdio-dev so it's now public. I'll mail the patch for this in a few minutes and then work toward a new release, although I still want 0.78.2-r2 as target stable ;)
public via http://lists.gnu.org/archive/html/libcdio-devel/2007-12/msg00009.html Arches, please test and mark stable dev-libs/libcdio-0.78.2-r2. Target keywords : "alpha amd64 arm hppa ia64 ppc ppc64 sh sparc x86" Already stabled : "amd64 hppa ppc x86" Missing keywords: "alpha arm ia64 ppc64 sh sparc"
alpha/ia64/sparc stable
request filed
(In reply to comment #0) > dev-libs/libcdio-0.78.2 provides iso-info and cd-info both exhibit this > vulnerability. > > A properly formatted iso image can produce a coredump and may allow for > malicious code injection. > > Reproducible: Always > > Steps to Reproduce: > 1. mkdir -p tmp/dir1 > 2. echo file_with_really_really_long_silly_name_to_test_iso_info_buffer > 3. mkisofs -J -R -volid My_Image -o test.iso tmp > 4. iso-info -l test.iso > > Actual Results: > iso-info lists files and directories up to the ver long file name at which > point it segfaults. > > Expected Results: > a listing of the directories > > in src/iso-info.c, in function "print_iso9660_recurse" there is a block of code > that begins: > > _CDIO_LIST_FOREACH (entnode, entlist) > { > iso9660_stat_t *p_statbuf = _cdio_list_node_data (entnode); > char *psz_iso_name = p_statbuf->filename; > char _fullname[4096] = { 0, }; > char translated_name[MAX_ISONAME+1]; > > > if (yep != p_statbuf->rr.b3_rock || 1 == opts.no_rock_ridge) { > iso9660_name_translate_ext(psz_iso_name, translated_name, > i_joliet_level); > snprintf (_fullname, sizeof (_fullname), "%s%s", psz_path, > translated_name); > } else { > snprintf (_fullname, sizeof (_fullname), "%s%s", psz_path, > psz_iso_name); > > > I believe either the test "yep != p_statbuf->rr.b3_rock" is incorrect, or > possibly the code in lib/iso9660 that sets "b3_rock". The field "b3_rock" is an > enumeration with the values "nope", "yep", "dunno". The test would be valid if > "b3_rock" was a boolean, but the additional "dunno" state causes a problem. > > When the iso is created with the options given above, the value of "b3_rock" is > "dunno" (integer 2). This causes the code to enter the > "iso9660_name_translate_ext" which writes beyond the end of "translated_name" > and corrupts the stack. > > I have also submitted this upstream as bug #21910 > (In reply to comment #5) > Created an attachment (id=139644) [edit] > libcdio-info-buffer.patch > > Upstream committed patch > Shouldn't the +1 go outside the strlen bracets? Likely won't cause a problem now that things are on the heap but... worth mentioning I think
(In reply to comment #19) > > char translated_name[MAX_ISONAME+1]; > > > > (In reply to comment #5) > > Created an attachment (id=139644) [edit] > > libcdio-info-buffer.patch > > > > Upstream committed patch > > > > Shouldn't the +1 go outside the strlen bracets? Likely won't cause a problem > now that things are on the heap but... worth mentioning I think > Yes, good catch, As it is, the alloca'd string may be 2 bytes too small.I have updated the upstream bug report with this information,
Created attachment 139967 [details, diff] Buffer patch with correction for +1 in the wrong place Moved the +1 outside the strlen call so the size is the length of the string plus 1, not the length of the string minus 1.
I think it the original version was by design: the translation strings remove ;1 suffix from filenames, so strlen(foo+1) is equivalent to writing strlen(foo)-1, just making strlen faster (afair, strlen() is O(n)).
Created attachment 140011 [details, diff] Upstream patch for misplaced +1 Patch from upstream cvs repo.
(In reply to comment #22) > I think it the original version was by design: the translation strings remove > ;1 suffix from filenames, so strlen(foo+1) is equivalent to writing > strlen(foo)-1, just making strlen faster (afair, strlen() is O(n)). Negative. Upstream has confirmed and patched. See attachment #140011 [details, diff]
media-video, your ball.
*** Bug 204333 has been marked as a duplicate of this bug. ***
Committed 0.78.2-r3 straight-to-stable for those who already stabled -r2.
Arches, please test and mark stable dev-libs/libcdio-0.78.2-r3. Target keywords : "alpha amd64 arm hppa ia64 ppc ppc64 sh sparc x86" Already stabled : "alpha amd64 hppa ia64 ppc ppc64 sparc x86" Missing keywords: "arm sh"
Stanislav Brabec from SUSE pointed out that allocating a larger string on the stack might easier lead to stack exhaustion, which can be shown by using some ten thousand files with long file names. Not sure on whether upstream will apply his patches, but would you consider this a regression (media-video/Diego)?
Diego, do you consider this a regression? If not, we'd glsa this now, otherwise we'll go stabling a new ebuild. The patch has been committed to the upstream CVS.
(In reply to comment #29) > Stanislav Brabec from SUSE pointed out that allocating a larger string on the > stack might easier lead to stack exhaustion, which can be shown by using some > ten thousand files with long file names. > Not sure on whether upstream will apply his patches, but would you consider > this a regression (media-video/Diego)? > I was looking at this scenario just recently. From the alloca() man page: RETURN VALUE The alloca() function returns a pointer to the beginning of the allo- cated space. If the allocation causes stack overflow, program behavior is undefined. In some cases I think this could cause some major problems even further security issues. In the simplest case this will only cause stability issues. My suggestion would be to use the heap rather than alloca().
Seems like we should go back to ebuild to get this fixed?
Sorry I was out of order because of the cold/fever, I'll see to handle this today.
0.78.2-r4 and 0.79-r1 are in tree. I didn't commit straight to stable this time, just for safety.
Thx Diego. Arches please test and mark stable. Target keywords are: libcdio-0.78.2-r4.ebuild:KEYWORDS="alpha amd64 arm hppa ia64 ppc ppc64 sh sparc ~sparc-fbsd x86 ~x86-fbsd"
ppc stable
x86 stable
Sparc stable.
alpha/ia64 stable
Stable for HPPA.
amd64 stable, [glsa] again.
GLSA 200801-08, thanks.
