Bug in commit 'Handle BER-encoded indefinite length values better' about asn1crypto HOT 7 OPEN

Today I discovered exactly the problem reported here.
I am not certain the proposed fix is correct. Looking at BER length encoding seems to me that the indefinite length encoding has the only length octet 0x80, but tag can be encoded on more octets, so the length octet can be on offset 1 or more... So the condition self._header[-1] == b'\x80' is basicaly correct, but must be completed by test, that preceding octet is not higher length octet...

from asn1crypto.

Can you provide an example of an encoded ASN.1 value that exposes this bug?

From my understanding the header can’t end in 0x80 for the DER encoding. See the high bit is set, but then the length is set to 0 since all seven lower bits are 0. This switches to indefinite length mode, which is used when streaming a value where the encoder doesn’t know the total length. As a result the decoder has to look for indefinite chunks. This by definition isn’t DER encoding since there can only be one canonical DER encoding. Instead this implied a BER encoding.

This is why I’d like an example so I can see what is actually going on.

from asn1crypto.

I mentioned the example in bug #195 already. See my last comments there. The certificate in the attached zip file... The highest bit you mentioned - this is about the short form (length up to 127). There can be a number of length octets in header for a length over 127. The last octet can be 0x80 quite easily.

from asn1crypto.

Posting an example on a different issue isn’t as helpful as posting it on the issue about the bug.

My recollection is fuzzy, but I thought the high bit was not set on any of the length bits? Or are you saying it is only not set on a single-byte-encoded length?

from asn1crypto.

Yeah, so that was most likely the source of the bug in the implementation. https://luca.ntop.org/Teaching/Appunti/asn1.html Confirms the high bit only matters on the first byte.

from asn1crypto.

For example I have the problem in this structure.
cms.zip
The second item have this header:
SEQUENCE (5 elem) - 30 82 0A 80
And last byte is 80, but this byte is last byte of length - 0x0A 0x80 of sequence.

from asn1crypto.

I am afraid that test can't be easily written in only one if statement. Identification/tag can occupy more than the first octet of the header. So self._header[1] is not necessarily the first length octet...

from asn1crypto.