Bugzilla Bug 203777

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 an attachment (id=139644) [details]
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

ppc64 done

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] [details]
> 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] [details]
> > 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 an attachment (id=139967) [details]
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 an attachment (id=140011) [details]
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]

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"

ppc64 done

ppc stable

x86 stable

Sparc stable.

alpha/ia64 stable

Stable for HPPA.

amd64 stable, [glsa] again.

GLSA 200801-08, thanks.