Hi Rocky,
The person who reported it to you originally found the issue in Rufus
(which I where I pointed them to libcdio), and I am planning to patch it
there when I get a chance, as well as send a proposal for a libcdio fix.
The most obvious fix, since that's where we overflow and, if we look at
the code that gets executed prior we know that the size of the data we
reserved for the filename is i_inlen + 2, is to replace:
strcpy(cpy_result, p_psz_out);
with
strncpy(cpy_result, p_psz_out, i_inlen);
As far as I can tell, we also shouldn't have to bother about NUL
terminating the string if cpy_result is longer, since we calloc'd the
i_inlen + 2, though I'm not completely sure I understand what is meant
by the comment at
https://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/iso9660/iso9660_fs.c#n860
which is where the extra 2 comes from:
/* .. string in statbuf is one longer than in p_iso9660_dir's listing
'\1' */
Testing with this change in Rufus seems to fix the overflow I saw on the
specially crafted image that was also provided to me. But I am still
planning to look into this a little bit further and I am also open to
suggestions or remarks on the best way to address this, as root of the
issue seems that we can produce UTF-8 strings that quite a bit longer
than the size of the filename buffer we allocated from looking at the
p_iso9660_dir->filename.len value in
https://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/iso9660/iso9660_fs.c#n858.
Regards,
/Pete
On 2024.04.05 00:51, Rocky Bernstein wrote:
I just received a report about a place in libiso9660
<https://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/iso9660/iso9660_fs.c#n814>
where we use strcpy() instead of strncpy().
If someone has a suggestion for how to fix, please let me know. I can send
a more detailed report for those interested. Just email me.
