Re: Bindat can exhaust memory when unpacking to vector

[Petteri Hintsanen <petterih@iki.fi>]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>   (defconst foo-bindat-spec
>>     (bindat-type
>>      (length uint 32)
>>      (_ unit (when (> length 1234) (error "malicious input")))
>>      (data vec length)))
>> 
>> Which is somewhat messy, but it works.
>
> You should also be able to do
>
>     (defconst foo-bindat-spec
>       (bindat-type
>        (length uint 32)
>        (data vec (if (< length 1234) length (error "malicious input")))))
>
> as well.  But it's still not very elegant.

Ah sure, right.  This is nice I think.

>     (defconst foo-bindat-spec
>       (bindat-type
>        (length uint 32)
>        (data vec (bounded 1 length 1234))))
>
> so we signal an error if the length is less than 1 or larger than 1234.

Sure.  But I'm not sure which kind of helpers would be the most
beneficial for general use.  Like said, I don't have that many examples,
and for my few validation cases I have found the "unit type" to be good
enough.

>> I also played around with the idea of patching bindat.el itself to do
>> trivial checking against the input data size, like this:
>> [...]
> Actually, this is a nice solution, I think.
> It seems hypothetical enough that I think we should go with your patch.

Feel free to apply it in whatever form you want, if you think it is
appropriate.

>> Checking should be optional and somehow programmable, perhaps
>> a separate "checked vec" type?  (I don't have any good, concrete
>> ideas, sorry.)
>
> I don't see the benefit of not-checking, to be honest.

I probably thought about efficiency here.  If bindat-unpack is hammered
many many many times and the checking could be _safely_ ignored, then,
perhaps, there might be a noticeable difference in performance.  Or
maybe not.

Thanks,
Petteri