esnacc / esnacc-ng

A continuation of the Enhanced SNACC ASN.1 compiler - See http://mail.esnacc.org/mailman/listinfo/dev
http://esnacc.org
GNU General Public License v2.0
32 stars 30 forks source link

BUG in decoding ANY DEFINED BY #46

Closed jarek556 closed 5 years ago

jarek556 commented 6 years ago

The following structure

ContentInfo ::= SEQUENCE {
       contentType ContentType,
       content [0] EXPLICIT ANY DEFINED BY contentType }

is not properly decoded with current esnacc, but works fine with old esnacc 1.7.4. It looks that current esnacc can't properly count size of NDEF BER encoded data:

  0 NDEF: SEQUENCE {
   2    9:   OBJECT IDENTIFIER signedData (1 2 840 113549 1 7 2)
  13 NDEF:   [0] {
  15 NDEF:     SEQUENCE {
  17    1:       INTEGER 1
  20   15:       SET {
  22   13:         SEQUENCE {
  24    9:           OBJECT IDENTIFIER sha-256 (2 16 840 1 101 3 4 2 1)
  35    0:           NULL
         :           }
         :         } 
-------------------------- here ConsStringDeck::Fill stops counting size
  37 NDEF:       SEQUENCE {
  39    9:         OBJECT IDENTIFIER data (1 2 840 113549 1 7 1)
  50 NDEF:         [0] {
  52  101:           OCTET STRING, encapsulates {
  54   99:             SEQUENCE {

The problematic change is in asn-octs.cpp ConsStringDeck::Fill:

619c614
<         for (; (curr != refList.end()) && ((curr->count < curr->length) || (curr->length == INDEFINITE_LEN));)
---
>         for (; (curr->count < curr->length) || (curr->length == INDEFINITE_LEN);)
711c706
<         if( curr != refList.begin() && curr != refList.end() )
---
>         if( curr != refList.begin() )
714,718c709,712
<             curr = refList.erase(curr);
<             if( curr != refList.end() )
<                 curr->count += iTmpCount;
<             else
<                 done = true;
---
>             refList.erase(curr);
>             curr = refList.end();
>             --curr;
>             curr->count += iTmpCount;
apconole commented 6 years ago

Thanks for the report. I'll gin up a test case and a fix.

apconole commented 6 years ago

Can you check and see if this is a problem with 1.8 branch as well?

jarek556 commented 6 years ago

1.8 - the same problem:

non zero byte in EOC or end of data reached
{ -- SEQUENCE --
    contentType { 1 2 840 113549 1 7 2},
    content UNKNOWN ANY hex dump: 0x30   0x80   0x2   0x1   0x1   0x31   0xf   
}
apconole commented 6 years ago

Great. Fixing it now. Also found some issues in the inttest code, and the cxx unit tests altogether, so I'm working on that as well. I'll post a link to the series when I have it complete.

apconole commented 6 years ago

Sorry this is taking so long. I've figured out how to re-enable the VDA tests, and have found bugs pre-dating 1.7 release that I'm having to resolve. Expect a large series (sadly...)

jarek556 commented 6 years ago

Any news ?

apconole commented 6 years ago

Sorry about this. I'll submit a bunch of patches to the mailing list by tomorrow EOB.

apconole commented 6 years ago

I submitted patches, but the list had been broken. Resubmitted at http://mail.esnacc.org/pipermail/dev/2018-August/000438.html you can grab the preliminary series at: http://patchwork.esnacc.org/series/7/mbox/

I just tested with the following test case:


ContentInfo ci;
unsigned char bad_bytes[] =
    { 0x30, /* UNIV, CONS, SEQ */
      0x18, /* Length: 17 */
      0x06, 0x06, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, /* OID */
      0xa0, 0x80,
      0x30, 0x80, 0x02, 0x01, 0x01, 0x31, 0x03, 0x02, 0x1, 0x2,
      0x00, 0x00,
      0x00, 0x00 };

AsnBuf b((char *)bad_bytes, sizeof(bad_bytes));
AsnLen l;
ci.BDec(b, l);

Please let me know if this resolves / doesn't resolve the issue.

apconole commented 6 years ago

Just wondering if you got a chance to test the patches out

apconole commented 6 years ago

Ping - if you could include a set of bytes I can throw it into the test suite in case this doesn't fix.

jarek556 commented 6 years ago

Hello!

Thank you for fix, I will check it in few days. I was very busy in last

time...

best regards Jarek

Dnia 2018-08-28, wto o godzinie 10:46 -0700, Aaron Conole pisze:

Ping - if you could include a set of bytes I can throw it into the test suite in case this doesn't fix.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

jarek556 commented 6 years ago

Hello! After your patch, compilation of test fails (debian stretch, g++ (Debian 6.3.0-18+deb9u1) 6.3.0 20170516): [....] cxx-examples/src/vdatestasn.cpp:230:51: error: qualified-id in declaration before ‘(’ token DirectoryString::PrintableString::SizeConstraints(int &sizeList)const ^ cxx-examples/src/vdatestasn.cpp:230:52: error: expected primary-expression before ‘int’ DirectoryString::PrintableString::SizeConstraints(int &sizeList)const ^~~ cxx-examples/src/vdatest_asn.cpp:237:34: error: expected primary-expression before ‘const’ DirectoryString::DirectoryString(const DirectoryString &that) ^~~~~ cxx-examples/src/vdatest_asn.cpp:237:61: error: cannot call constructor ‘SNACC::DirectoryString::DirectoryString’ directly [-fpermissive] DirectoryString::DirectoryString(const DirectoryString &that) ^ cxx-examples/src/vdatest_asn.cpp:237:61: note: for a function-style cast, remove the redundant ‘::DirectoryString’ cxx-examples/src/vdatest_asn.cpp:242:27: error: qualified-id in declaration before ‘(’ token void DirectoryString::Init(void) ^ cxx-examples/src/vdatest_asn.cpp:250:38: error: qualified-id in declaration before ‘(’ token int DirectoryString::checkConstraints(ConstraintFailList* pConstraintFails) const ^ cxx-examples/src/vdatest_asn.cpp:271:28: error: qualified-id in declaration before ‘(’ token void DirectoryString::Clear() ^ cxx-examples/src/vdatest_asn.cpp:3229:1: error: expected ‘}’ at end of input } // namespace close ^ cxx-examples/src/vdatest_asn.cpp: At global scope: cxx-examples/src/vdatest_asn.cpp:3229:1: error: expected ‘}’ at end of input Dnia wt, sie 28, 2018 o 19:46, Aaron Conole napisał: Ping - if you could include a set of bytes I can throw it into the test suite in case this doesn't fix. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub (https://github.com/esnacc/esnacc-ng/issues/46#issuecomment-416678551), or mute the thread (https://github.com/notifications/unsubscribe-auth/AM2EmKsIlhhSlvuMngjqmdKmr-Ym1VLKks5uVYIFgaJpZM4T5cvc).

apconole commented 6 years ago

d'oh! Okay, I'll pull a VM image and respin it to work. Thanks for the attempt.

apconole commented 6 years ago

whoops, you'll need: http://patchwork.esnacc.org/patch/35/mbox/

That will fix the issue. Sorry for that. I intended to submit it with the series, but looks like I selected the wrong range.

apconole commented 6 years ago

Any update?

apconole commented 5 years ago

I've pushed those patches upstream. If the issue still exists in the latest master, please re-open.