Re: [Cellar] Security considerations: recursive elements

[Jerome Martinez <[e-mail]>]

On 17/01/2018 23:47, Michael Bradshaw wrote:
> Thanks for the responses, all!
>
> I agree that setting minimum/maximum recursion limits doesn't belong 
> in the EBML spec. I do think the EBML spec should have a note in the 
> Security Considerations that EBML Readers need to be cognizant of how 
> they handle recursive elements

I missed the case of recursive elements, specified in EBML.
for a security section, I don't think that talking about a parser is 
right, reading https://tools.ietf.org/html/rfc3552 it seems to more 
about usage of the correct behavior of a protocol rather than badly 
implemented parser, I am more in favor of  something like "it is 
possible to have unlimited recursion, potential source of denial of 
service attacks" in security section and
"An implementation may set limits on the maximum depth of nesting" in a 
parser section (similar to JSON RFC).

> and should be resilient to deeply nested recursion attacks (without 
> giving particular recommendations for how that resilience is 
> achieved). But EBML itself shouldn't attempt to set any specific 
> limits on recursion depth.
>
> The reason I suggested setting some kind of minimum/maximum recursion 
> depth (for Matroska specifically, not for EBML in general) is because 
> of the points Michael Niedermayer raises. Setting a minimum allows 
> muxers to write files with confidence that readers will be able to 
> use/play the data being written (so long as the minimum isn't exceeded).

We don't talk anymore about security, and confidence depends on the 
underlying document e.g. for my EBML document x, I may need only 1 level 
beside the EBML header and it is fine with that, no need of more.
So:
- A minimum should not be in Security section (it is not about security)
- A minimum depends on the goal of the parser, e.g. a parser just 
checking top level element size and CRC for checking a file transfer is 
fine does not need more than 2 levels. Not all tools are video players.

> Setting a maximum let's muxers know that if they exceed it, they run 
> the risk of making a file that some readers can't (fully) play.

As far as I know, there is nothing mandatory: tags are not mandatory, a 
specific stream format is not mandatory, it would be a full new part if 
we say what a reader should play, with lot of debates (chapters 
mandatory? tags display mandatory? FFV1 mandatory? H.265 and its 
royalties mandatory? NTSC only or 8K? Subtitles support mandatory? color 
description mandatory? maximum length of a text in a tag? can we have a 
gap in streams?).
IMO we talk about a file format and not about what a reader expects to 
fully play.

>
> Specifying recursion limits using the example language in my first 
> email ("a parser SHOULD handle recursion up to X levels deep, and MAY 
> abort the parse if it reaches Y levels deep") for ChapterAtom and 
> SimpleTag elements would allow a muxing program to emit a warning to 
> the user if the recursion level exceeded Y (it's not a fatal error, 
> but it would be nice to let the user know their file has abnormally 
> deep recursion that might not be compatible with various parsers).

But a player may just ignore any chapter and tag elements and still play 
fine the file.
IMO expected supported elements of Matroska is a different document 
(e.g. a document about a reader capability? I usually see that with a 
"format compliance program", having lot of sample files), not in format 
description which is more general and does not say what element is 
mandatory, just how to store it.
I don't say that it is a bad idea, just that I don't think that saying 
minimal requirements or warnings about potentially unsupported nesting 
level should be in a bitstream specification permitting lot of optional 
parts.

Jérôme