Hi,
* Nico Golde <[EMAIL PROTECTED]> [2008-01-05 15:14]:
> during some reading in libcdio I found a bug in the iso9660_dir_to_name
> function.
>
> 855 char *
> 856 iso9660_dir_to_name (const iso9660_dir_t *iso9660_dir)
> 857 {
> 858 char namebuf[256] = { 0, };
> 859 uint8_t len=iso9660_get_dir_len(iso9660_dir);
> 860
> 861 if (!len) return NULL;
> 862
> 863 cdio_assert (len >= sizeof (iso9660_dir_t));
> 864
> 865 /* (iso9660_dir->file_flags & ISO_DIRECTORY) */
> 866 ยทยท
> 867 if (iso9660_dir->filename[0] == '\0')
> 868 strncpy (namebuf, ".", sizeof("."));
> 869 else if (iso9660_dir->filename[0] == '\1')
> 870 strncpy (namebuf, "..", sizeof(".."));
> 871 else
> 872 strncpy (namebuf, iso9660_dir->filename, iso9660_dir->filename_len);
> 873
> 874 return strdup (namebuf);
> 875 }
>
> In line 863 there is check for the size of the directory length. It checks
> whether it's
> bigger than the iso9660_dir_t struct which is basically iso9660_dir_s. I did
> not check
> the exact size but it's a rather huge structure.
>
> Then in line 872 it copies iso9660_dir->filename to namebuf and uses
> iso9660_dir->filename_len as length modifier. This check is wrong.
> It should check sizeof(namebuf) instead to prevent a stack-based buffer
> overflow here.
[...]
It turned out that this is not a vulnerability but just bad
programming :) Since len is of type uint8_t it can have
values ranging from 0 to 255 so the assert would actually
never fail even if len is bigger because then it will be
truncated to uint8_t again. But unfortunately
iso9660_dir->filename_len is also of type uint8_t so copy
operation is ok even if it makes not much sense. The only
thing that would work is to produce a buffer overflow on
iso creation because of the uint8_t truncation.Thus closing the bug. Kind regards Nico -- Nico Golde - http://www.ngolde.de - [EMAIL PROTECTED] - GPG: 0x73647CFF For security reasons, all text in this mail is double-rot13 encrypted.
pgp9B8BhCiB8N.pgp
Description: PGP signature
