From 796c1da2cb73f8304876d0819a5f2619c5d917d8 Mon Sep 17 00:00:00 2001 From: vlm Date: Wed, 21 Jul 2004 03:55:44 +0000 Subject: [PATCH] invalid memory reference fix and test case git-svn-id: https://asn1c.svn.sourceforge.net/svnroot/asn1c/trunk@56 59561ff5-6e30-0410-9f3c-9617f08c8826 --- ChangeLog | 14 +++++- asn1c/tests/check-48.c | 96 ++++++++++++++++++++++++++++++++++++++ skeletons/ber_decoder.c | 33 +++++++------ skeletons/der_encoder.c | 16 +++++-- tests/48-real-life-OK.asn1 | 21 +++++++++ 5 files changed, 162 insertions(+), 18 deletions(-) create mode 100644 asn1c/tests/check-48.c create mode 100644 tests/48-real-life-OK.asn1 diff --git a/ChangeLog b/ChangeLog index cb6b9d4b..3a654a54 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,8 +1,20 @@ -0.8.15: 2004-Jul-15 +0.8.15: 2004-Jul-20 * Fixed parser: memory leak in free_struct code for SET OF/SEQUENCE OF. (Severity: high, Security impact: medium) + * Fixed parser: invalid memory reference in CHECK. + (Test case 48) (Severity: high, Security impact: medium) + When encoding data for certain ASN.1 specifications containing + explicit tags, the tag is always written incorrectly due to + incorrect memory reference. The encoding will almost always produce + unparseable data and might well reference unmapped region so program + would produce segmentation violation. Fortunately, memory is + read, not written, so remote exploits cannot execute arbitrary + code and triggering unmapped memory reference is highly unlikely + even it attacker knows the code (basically, the compiler should place + asn1_DEF_... right before the end of the mapped memory region, which + is extremely rare). * Improved INTEGER type printing. 0.8.14: 2004-Jun-30 diff --git a/asn1c/tests/check-48.c b/asn1c/tests/check-48.c new file mode 100644 index 00000000..8c4f395f --- /dev/null +++ b/asn1c/tests/check-48.c @@ -0,0 +1,96 @@ +#undef NDEBUG +#include +#include +#include +#include +#include +#include + +#include + +static unsigned char buf[4096]; +static int buf_offset; + +static int +_buf_writer(const void *buffer, size_t size, void *app_key) { + unsigned char *b, *bend; + (void)app_key; + assert(buf_offset + size < sizeof(buf)); + memcpy(buf + buf_offset, buffer, size); + b = buf + buf_offset; + bend = b + size; + printf("=> ["); + for(; b < bend; b++) + printf(" %02X", *b); + printf("]:%ld\n", (long)size); + buf_offset += size; + return 0; +} + +static int +save_object(void *bs, asn1_TYPE_descriptor_t *td) { + der_enc_rval_t rval; /* Return value */ + int i; + + rval = der_encode(td, bs, _buf_writer, 0); + if (rval.encoded == -1) { + fprintf(stderr, + "Cannot encode %s: %s\n", + rval.failed_type->name, strerror(errno)); + assert(rval.encoded != -1); + return -1; /* JIC */ + } + + buf[buf_offset++] = 123; /* Finalize with garbage */ + + asn_fprint(stderr, td, bs); + + printf("OUT: ["); + for(i = 0; i < buf_offset; i++) + printf(" %02x", buf[i]); + printf("]\n"); + + return 0; +} + +static int +load_object(void *bs, asn1_TYPE_descriptor_t *td) { + ber_dec_rval_t rval; + + fprintf(stderr, "\nLOADING OBJECT OF SIZE %d\n", buf_offset); + + rval = ber_decode(td, (void **)&bs, buf, buf_offset); + assert(rval.code == RC_OK); + + asn_fprint(stderr, td, bs); + + return (rval.code == RC_OK)?0:-1; +} + +int +main() { + asn1_TYPE_descriptor_t *td = &asn1_DEF_UserIdentifier; + UserIdentifier_t user; + UserIdentifier_t user_new; + int ret; + + memset(&user, 0, sizeof user); + memset(&user_new, 0, sizeof user_new); + + user.present = UserIdentifier_PR_phoneNumber; + OCTET_STRING_fromBuf( + &user.choice.phoneNumber, + "0123456789", -1); + + /* Save->Load must succeed */ + save_object(&user, td); + ret = load_object(&user_new, td); + + assert(user_new.present == UserIdentifier_PR_phoneNumber); + + assert(ret == 0); + + printf("OK\n"); + + return ret; +} diff --git a/skeletons/ber_decoder.c b/skeletons/ber_decoder.c index ab8d671c..0d57f82c 100644 --- a/skeletons/ber_decoder.c +++ b/skeletons/ber_decoder.c @@ -39,7 +39,7 @@ ber_decode(asn1_TYPE_descriptor_t *type_descriptor, * Check the set of >> tags matches the definition. */ ber_dec_rval_t -ber_check_tags(asn1_TYPE_descriptor_t *head, ber_dec_ctx_t *ctx, +ber_check_tags(asn1_TYPE_descriptor_t *td, ber_dec_ctx_t *ctx, void *ptr, size_t size, int tag_mode, ber_tlv_len_t *last_length, int *opt_tlv_form) { ssize_t consumed_myself = 0; @@ -83,28 +83,33 @@ ber_check_tags(asn1_TYPE_descriptor_t *head, ber_dec_ctx_t *ctx, * This is because the implicit tag at above structure may replace * zero or more (or every) tags which follow it. We don't care * about the precise number, as it is already computed for us - * by the ASN.1 compiler and placed into head->tags_impl_skip. + * by the ASN.1 compiler and placed into td->tags_impl_skip. * So let's suppose the only tag left after implicit tagging is {I}. - * Yet, the table we have is {A,B,C} and head->tags_impl_skip=3. + * Yet, the table we have is {A,B,C} and td->tags_impl_skip=3. * We need to check at least one tag in the loop, so the loop range * is modified so it will be invoked at least one time. */ tagno = ctx->step /* Continuing where left previously */ - + (tag_mode==-1?(head->tags_impl_skip-1):0) + + (tag_mode==-1?(td->tags_impl_skip-1):0) + (tag_mode==1?-1:0) ; - //assert(head->tags_count >= 1); ?May not be the case for CHOICE! - assert(tagno < head->tags_count); /* At least one loop */ - for((void)tagno; tagno < head->tags_count; tagno++, ctx->step++) { + ASN_DEBUG("ber_check_tags(%s, size=%ld, tm=%d, step=%d, tagno=%d)", + td->name, (long)size, tag_mode, ctx->step, tagno); + //assert(td->tags_count >= 1); ?May not be the case for CHOICE! + assert(tagno < td->tags_count); /* At least one loop */ + for((void)tagno; tagno < td->tags_count; tagno++, ctx->step++) { /* * Fetch and process T from TLV. */ tag_len = ber_fetch_tag(ptr, size, &tlv_tag); - ASN_DEBUG("Fetching tag from {%p,%ld} %02X: " + ASN_DEBUG("Fetching tag from {%p,%ld} %02X..%02X: " "len %ld, tag %s", ptr, (long)size, - *(uint8_t *)ptr, (long)tag_len, + size?*(uint8_t *)ptr:0, + (tag_len0) + ?*((uint8_t *)ptr + tag_len):0, + (long)tag_len, ber_tlv_tag_string(tlv_tag)); switch(tag_len) { case -1: RETURN(RC_FAIL); @@ -125,12 +130,12 @@ ber_check_tags(asn1_TYPE_descriptor_t *head, ber_dec_ctx_t *ctx, */ } else { assert(tagno >= 0); /* Guaranteed by the code above */ - if(tlv_tag != head->tags[tagno]) { + if(tlv_tag != td->tags[tagno]) { /* * Unexpected tag. Too bad. */ ASN_DEBUG("Expected: %s, expectation failed", - ber_tlv_tag_string(head->tags[tagno])); + ber_tlv_tag_string(td->tags[tagno])); RETURN(RC_FAIL); } } @@ -142,13 +147,13 @@ ber_check_tags(asn1_TYPE_descriptor_t *head, ber_dec_ctx_t *ctx, * If this one is the last one, check that the tag form * matches the one given in descriptor. */ - if(tagno < (head->tags_count - 1)) { + if(tagno < (td->tags_count - 1)) { if(tlv_constr == 0) { RETURN(RC_FAIL); } } else { - if(head->last_tag_form != tlv_constr - && head->last_tag_form != -1) { + if(td->last_tag_form != tlv_constr + && td->last_tag_form != -1) { RETURN(RC_FAIL); } } diff --git a/skeletons/der_encoder.c b/skeletons/der_encoder.c index 72a33ebc..919350c5 100644 --- a/skeletons/der_encoder.c +++ b/skeletons/der_encoder.c @@ -43,22 +43,32 @@ der_write_tags(asn1_TYPE_descriptor_t *sd, ssize_t *lens; int i; + ASN_DEBUG("Writing tags (%s, tm=%d, tc=%d, iskip=%d, tag=%s, mtc=%d)", + sd->name, tag_mode, sd->tags_count, sd->tags_impl_skip, + ber_tlv_tag_string(tag), + tag_mode + ?(sd->tags_count+1 + -((tag_mode==-1)?sd->tags_impl_skip:0)) + :sd->tags_count + ); + if(tag_mode) { /* * Instead of doing shaman dance like we do in ber_check_tags(), * allocate a small array on the stack * and initialize it appropriately. */ - tags = alloca((sd->tags_count + (tag_mode?1:0)) - * sizeof(ber_tlv_tag_t)); + int stag_offset; + tags = alloca((sd->tags_count + 1) * sizeof(ber_tlv_tag_t)); if(tags == NULL) return -1; /* Impossible on i386 */ tags_count = sd->tags_count + 1 /* EXPLICIT or IMPLICIT tag is given */ - ((tag_mode==-1)?sd->tags_impl_skip:0); /* Copy tags over */ tags[0] = tag; + stag_offset = -1 + ((tag_mode==-1)?sd->tags_impl_skip:0); for(i = 1; i < tags_count; i++) - tags[i] = sd->tags[i - 1 + sd->tags_impl_skip]; + tags[i] = sd->tags[i + stag_offset]; } else { tags = sd->tags; tags_count = sd->tags_count; diff --git a/tests/48-real-life-OK.asn1 b/tests/48-real-life-OK.asn1 new file mode 100644 index 00000000..71736761 --- /dev/null +++ b/tests/48-real-life-OK.asn1 @@ -0,0 +1,21 @@ + +-- OK: Everything is fine + +-- iso.org.dod.internet.private.enterprise (1.3.6.1.4.1) +-- .spelio.software.asn1c.test (9363.1.5.1) +-- .48 + +ModuleSetChoiceExtensibility + { iso org(3) dod(6) internet (1) private(4) enterprise(1) + spelio(9363) software(1) asn1c(5) test(1) 48 } + DEFINITIONS ::= +BEGIN + + /* + * 0.8.14 had problem saving/reloading this object + */ + UserIdentifier ::= CHOICE { + phoneNumber [3] EXPLICIT IA5String + } + +END