libcdio-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Libcdio-devel] Rock Ridge and libisofs/xorriso 'AL' extension


From: Rocky Bernstein
Subject: Re: [Libcdio-devel] Rock Ridge and libisofs/xorriso 'AL' extension
Date: Sat, 29 Jul 2017 19:51:21 -0400

Comments in line...

On Fri, Jul 28, 2017 at 10:45 AM, Thomas Schmitt <address@hidden> wrote:

> Hi,
>
> if Rocky agrees that
>   https://savannah.gnu.org/bugs/?45015
> can be solved by changing
>   offset++;
> to
>   offset+= ISO_BLOCKSIZE - (offset % ISO_BLOCKSIZE);
> and that the whitelist of SUSP signatures can then be dropped,
> there remain two issues which made the further wrong consequences
> of the bug possible.
>
> I am not sure whether they need action. But i think they should be
> named and described here:
>
> 1:
>
> libcdio did not notice that the mistaken directory record (caused by the
> stray byte with value 128) reached over the end of the current block.
>

This is fixed in the patch you offer, right?


>
> One could add such a check to increase security against bad ISOs
> like the one of bug 45015.
>
> 2:
>

This too is fixed in the patch you offer, right?


>
> libcdio seems to assume a valid chain of SUSP fields when it encounters
> additional space after the end of ISO 9660 file name and possible padding
> byte. But the existence of System Use Area does not imply that there is
> the SUSP protocol inside.
>
> I got a link to SUSP-1.10 from Ady Ady whom i know from the SYSLINUX list:
>   https://archive.org/details/enf_pobox_Susp
> Already this version, which is the earliest known, prescribes mandatorily
> the presence of field "SP" in the first directory record of the root
> directory (that's the "." directory record).
>
> So actually one should not assume the byte-wise definition of SUSP fields
>
>   SIG1, SIG2, LENGTH, VERSION, DATA(1), ..., DATA(LENGTH - 4)
>
> unless the "SP" field was found at its fixely defined position, namely
> byte offset 34 and 35 of the block of which the number is recorded
> as little-endian 32-bit number at byte offset 32768+158 from the
> start of the ISO session.
> The byte sequence must be
>
>   {'S', 'P', 7, 1, 0xbe, 0xef, LEN_SKP }
>
> where any value of the LEN_SKP byte other than 0 would be an unusual
> challenge. SUSP-1.10 and 1.12 say:
>
>   "BP 7 - Bytes Skipped (LEN_SKP)" shall specify as an 8-bit number
>   the number of bytes to be skipped within the System Use Area of
>   each Directory Record (except the "." entry of the root directory)
>   before recording System Use Fields other than the "SP" field.
>
> I am not aware of any ISO which has non-zero as LEN_SKP.
> So i doubt that provisions for that are tested, if they exist at all.
>
> Further, RRIP-1.10 prescribes the existence of an "ER" field with
> Extension Identifier "RRIP_1991A".
> RRIP-1.12 prescribes "IEEE_P1282" instead.
> There are also RRIP-1.12 documents which say "IEEE_1282" (without a "P")
> All three should be considered valid.
>
> The "ER" filed has to be among the SUSP fields of the first directory
> record of the root directory. I.e. in the same record which bears the
> "SP" record or in its SUSP Continuation Area.
>
> Normally i would strongly urge to first check for "SP" indicating SUSP,
> and only then for an "ER", which indicates Rock Ridge, and only then
> enable reading of the System Use Area when accessing directories in
> that ISO.
>
> But to my assessment, the Linux kernel is quite as tolerant towards
> missing "SP" or "ER" with one of above Extension Identifiers.
> libcdio could well declare this as its guideline.
> In that case i would put more emphasis on a check against block limit
> crossing.
>

I'm cool with whatever you suggest. If we do one way and it's not
sufficient then
we it can always be changed in the future. SImilarly, if you don't want to
have
to deal with ever again by adding more work up front, that's okay too.


>
>
> Have a nice day :)
>
> Thomas
>
>
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]