I'll answer my own question by rereading better. My understanding is that these two issues still would need resolution.
On Sat, Jul 29, 2017 at 7:51 PM, Rocky Bernstein <[email protected]> wrote: > Comments in line... > > On Fri, Jul 28, 2017 at 10:45 AM, Thomas Schmitt <[email protected]> > 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 >> >> >> >
