pc-dart
pc-dart copied to clipboard
Is the asn1utils value start position calculated correctly?
Hi, Shouldn't the condition be rather less or equal here? https://github.com/bcgit/pc-dart/blob/7aebcf232f047bb3a4f549ca0b53f14da40d90b8/lib/asn1/asn1_utils.dart#L17
Similarly as in decodeLength() method below.
@mwcw @AKushWarrior He seems to be right I think. Can you double check it? After that we can create a PR.
Hi,
It is calculating the position of the start of the data.
However it is making an assumption that the tag is only one byte long, tags can be longer, this should be a TODO.
MW
Hi again, I'll add some background.
I've observed the issue when using https://pub.dev/packages/basic_utils package which lately migrated from asn1lib to pointycastle library.
The issue was with parsing one of my PEM files which has exactly 127 bytes long sequence inside. The effect was that the parser was trying to read data outside the buffer.
Quick fix which worked for this pem was to change the condition above to inclusive.
@domindx Can you provide this PEM so I can add this later as a unit tests to my "basic utils" package?
Some background information: I tested the parsing of PEM files with nearly 50k certificates issued by DigiCert. So therefore I am quite sure it works in 99% of all cases ;).
@Ephenodrom Sorry for lack of response, I was quite busy lately.
Unfortunately I cannot share the PEM, but I'll try to generate some example on which it may fail.
One thing which I want to add is that basic_utils version which is using asn1lib is working perfectly with mentioned PEM. Do you know the reasons why basic_utils migrated to pointycastle?
@domindx The main reason why I switched from asn1lib to Pointycastle was to cut down the amount of dependencies of the basic utils package.
I completely checked the new ASN1 implementation with all unit tests i had in the basic utils package and I wrote a lot of new unit tests in the Pointycastle package.
But as always, there is one special scenario that is not yet tested ...
Hello again, I wasn't able to prepare faulty PEM file, but I've added UT scenario which cover the issue I am encountering.
So in my case when the sequence have 7F length the start position seems to be calculated in the way that library is trying to read data outside the buffer.
From my understanding the start position in this case should be the same as for length below 7F. And actually it worked for me when I've made such change.
Please have a look at my commit: 61120af1ccb9a22a67895797c77a16a5818c4e45
@mwcw I created a PR to fix the calculation of the start position.
@mwcw Any information about the release date of the next version :)?