aper_support.c error parsing BMPString

[KIN92]

Hello, I I found a discrepancy in the logic of the parser. In aper_support.c, lines 143, 153, and 177 need to remove the return value check and just do the alignment, the accum value is not important. I realized this by analyzing the Wireshark source code; after the correction, the parser works correctly. I will attach an archive with a brief visual description. If you need asn modules, they are in the OCTET_STRING_aper.c error discussion https://github.com/mouse07410/asn1c/issues/190 Thank you!

https://transfiles.ru/eltsz

[mouse07410]

I am concerned about removing the return value check - what are the consequences of ignoring it, and if the return value does not matter - why is it returned at all? Any thoughts?

[KIN92]

I also ask myself these questions, but unfortunately I don’t have time to study aper so deeply. However, I corrected it in my version; in this case, Wireshark became a powerful argument for me. https://github.com/boundary/wireshark/blob/master/epan/dissectors/packet-per.c#L125 https://github.com/boundary/wireshark/blob/master/epan/dissectors/packet-per.c#L1268 https://github.com/boundary/wireshark/blob/master/epan/dissectors/packet-per.c#L1278 https://github.com/boundary/wireshark/blob/master/epan/dissectors/packet-per.c#L1303 It may be necessary to remove the check in all cases of using aper_get_align in aper_support.c. Unfortunately, I don’t have a large amount of encoded data to check all the options.

[mouse07410]

I also ask myself these questions, but unfortunately I don’t have time to study aper so deeply . . . . . . It may be necessary to remove the check in all cases of using aper_get_align in aper_support.c

Well, unless I'm given convincing evidence that removal of those checks does not open an exploitable hole, I cannot remove them. They were placed there for a reason (though not by me). _I've looked into per_get_few_bits() function, and it looks that only return of a negative value indicates a problem. So, I'm relaxing that test. Please check the vlm_master and let me know if it remediates your problem._

[KIN92]

I checked on my data, you are right, the parser works correctly, most likely only with a negative value it is necessary to interrupt the work.

[mouse07410]

Excellent - thanks for the report, and I'm glad the fix worked.

Closing this issue - please re-open if this fix https://github.com/mouse07410/asn1c/commit/998e7ea24d6a33ca42b45b667b5e42ab4f44a4bc is not complete.