From 2d59b5f412e0eacf4c89008b14df8251a92cae09 Mon Sep 17 00:00:00 2001 From: Tom Yu Date: Tue, 8 Oct 2002 02:27:39 +0000 Subject: [PATCH] ASN.1 code passes uninitialized values around * asn1_get.c (asn1_get_tag_indef): Stomp on asn1class, construction, retlen, and indef, even if we've hit the end of the buffer, to avoid passing uninitialized values around. * asn1_k_decode.c: Reformat somewhat and add comments to demystify things a little. (opt_field): Fix to explicitly check for end of subbuf before verifying the pre-fetched tag, which may have been stomped on by asn1_get_tag_indef() encountering end-of-buffer. * krb5_decode.c (opt_field, opt_lenfield): Fix to explicitly check for end of subbuf before verifying the pre-fetched tag, which may have been stomped on by asn1_get_tag_indef() encountering end-of-buffer. ticket: new target_version: 1.3 git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@14913 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/asn.1/ChangeLog | 17 ++ src/lib/krb5/asn.1/asn1_get.c | 12 +- src/lib/krb5/asn.1/asn1_k_decode.c | 319 ++++++++++++++++++----------- src/lib/krb5/asn.1/krb5_decode.c | 28 ++- 4 files changed, 239 insertions(+), 137 deletions(-) diff --git a/src/lib/krb5/asn.1/ChangeLog b/src/lib/krb5/asn.1/ChangeLog index 28ff9822a..db2fe2508 100644 --- a/src/lib/krb5/asn.1/ChangeLog +++ b/src/lib/krb5/asn.1/ChangeLog @@ -1,3 +1,20 @@ +2002-10-07 Tom Yu + + * asn1_get.c (asn1_get_tag_indef): Stomp on asn1class, + construction, retlen, and indef, even if we've hit the end of the + buffer, to avoid passing uninitialized values around. + + * asn1_k_decode.c: Reformat somewhat and add comments to demystify + things a little. + (opt_field): Fix to explicitly check for end of subbuf before + verifying the pre-fetched tag, which may have been stomped on by + asn1_get_tag_indef() encountering end-of-buffer. + + * krb5_decode.c (opt_field, opt_lenfield): Fix to explicitly check + for end of subbuf before verifying the pre-fetched tag, which may + have been stomped on by asn1_get_tag_indef() encountering + end-of-buffer. + 2002-09-02 Ken Raeburn * asn1_decode.c, asn1_encode.c, asn1_get.c, asn1_get.h, diff --git a/src/lib/krb5/asn.1/asn1_get.c b/src/lib/krb5/asn.1/asn1_get.c index 47dd2835b..e90ac6b06 100644 --- a/src/lib/krb5/asn.1/asn1_get.c +++ b/src/lib/krb5/asn.1/asn1_get.c @@ -35,8 +35,16 @@ asn1_get_tag_indef(asn1buf *buf, asn1_class *asn1class, if (buf == NULL || buf->base == NULL || buf->bound - buf->next + 1 <= 0) { - *tagnum = ASN1_TAGNUM_CEILING; - return 0; + *tagnum = ASN1_TAGNUM_CEILING; /* emphatically not an EOC tag */ + if (asn1class != NULL) + *asn1class = UNIVERSAL; + if (construction != NULL) + *construction = PRIMITIVE; + if (retlen != NULL) + *retlen = 0; + if (indef != NULL) + *indef = 0; + return 0; } retval = asn1_get_id(buf,asn1class,construction,tagnum); if(retval) return retval; diff --git a/src/lib/krb5/asn.1/asn1_k_decode.c b/src/lib/krb5/asn.1/asn1_k_decode.c index b1d47df6c..78d7e4725 100644 --- a/src/lib/krb5/asn.1/asn1_k_decode.c +++ b/src/lib/krb5/asn.1/asn1_k_decode.c @@ -29,141 +29,212 @@ #include "asn1_get.h" #include "asn1_misc.h" -#define setup()\ -asn1_error_code retval;\ -asn1_class asn1class;\ -asn1_construction construction;\ -asn1_tagnum tagnum;\ -unsigned int length,taglen - -#define unused_var(x) if(0) {x=0; x=x-x;} - -#define next_tag()\ -retval = asn1_get_tag_indef(&subbuf,&asn1class,&construction,\ - &tagnum,&taglen,&indef);\ -if(retval) return retval; - -#define get_eoc() \ -retval = asn1_get_tag_indef(&subbuf,&asn1class,&construction, \ - &tagnum,&taglen,&indef); \ -if(retval) return retval; \ -if(asn1class != UNIVERSAL || tagnum || indef) \ - return ASN1_MISSING_EOC - -#define alloc_field(var,type)\ -var = (type*)calloc(1,sizeof(type));\ -if((var) == NULL) return ENOMEM - - -#define apptag(tagexpect)\ -retval = asn1_get_tag(buf,&asn1class,&construction,&tagnum,&applen);\ -if(retval) return retval;\ -if(asn1class != APPLICATION || construction != CONSTRUCTED ||\ - tagnum != (tagexpect)) return ASN1_BAD_ID +/* Declare useful decoder variables. */ +#define setup() \ + asn1_error_code retval; \ + asn1_class asn1class; \ + asn1_construction construction; \ + asn1_tagnum tagnum; \ + unsigned int length, taglen + +#define unused_var(x) if (0) { x = 0; x = x - x; } + +/* This is used for prefetch of next tag in sequence. */ +#define next_tag() \ + retval = asn1_get_tag_indef(&subbuf, &asn1class, &construction, \ + &tagnum, &taglen, &indef); \ + if (retval) return retval; + +/* Force check for EOC tag. */ +#define get_eoc() \ + retval = asn1_get_tag_indef(&subbuf, &asn1class, &construction, \ + &tagnum, &taglen, &indef); \ + if (retval) return retval; \ + if (asn1class != UNIVERSAL || tagnum || indef) \ + return ASN1_MISSING_EOC + +#define alloc_field(var, type) \ + var = (type*)calloc(1, sizeof(type)); \ + if ((var) == NULL) return ENOMEM + +/* Fetch an expected APPLICATION class tag and verify. */ +#define apptag(tagexpect) \ + retval = asn1_get_tag(buf, &asn1class, &construction, &tagnum, &applen); \ + if (retval) return retval; \ + if (asn1class != APPLICATION || construction != CONSTRUCTED || \ + tagnum != (tagexpect)) return ASN1_BAD_ID /**** normal fields ****/ -#define get_field_body(var,decoder)\ -retval = decoder(&subbuf,&(var));\ -if(retval) return retval;\ -if(!taglen && indef) { get_eoc(); }\ -next_tag() - -#define get_field(var,tagexpect,decoder)\ -if(tagnum > (tagexpect)) return ASN1_MISSING_FIELD;\ -if(tagnum < (tagexpect)) return ASN1_MISPLACED_FIELD;\ -if((asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ - && (tagnum || taglen || asn1class != UNIVERSAL)) \ - return ASN1_BAD_ID;\ -get_field_body(var,decoder) - -#define opt_field(var,tagexpect,decoder,optvalue)\ -if((asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ - && (tagnum || taglen || asn1class != UNIVERSAL)) \ - return ASN1_BAD_ID;\ -if(tagnum == (tagexpect)){\ - get_field_body(var,decoder); }\ -else var = optvalue +/* + * get_field_body + * + * Get bare field. This also prefetches the next tag. The call to + * get_eoc() assumes that any values fetched by this macro are + * enclosed in a context-specific tag. + */ +#define get_field_body(var, decoder) \ + retval = decoder(&subbuf, &(var)); \ + if (retval) return retval; \ + if (!taglen && indef) { get_eoc(); } \ + next_tag() + +/* + * get_field + * + * Get field having an expected context specific tag. This assumes + * that context-specific tags are monotonically increasing in its + * verification of tag numbers. + */ +#define get_field(var, tagexpect, decoder) \ + if (tagnum > (tagexpect)) return ASN1_MISSING_FIELD; \ + if (tagnum < (tagexpect)) return ASN1_MISPLACED_FIELD; \ + if ((asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ + && (tagnum || taglen || asn1class != UNIVERSAL)) \ + return ASN1_BAD_ID; \ + get_field_body(var,decoder) + +/* + * opt_field + * + * Get an optional field with an expected context specific tag. + * Assumes that OPTVAL will have the default value, thus failing to + * distinguish between absent optional values and present optional + * values that happen to have the value of OPTVAL. + */ +#define opt_field(var, tagexpect, decoder, optvalue) \ + if (asn1buf_remains(&subbuf, seqindef)) { \ + if ((asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ + && (tagnum || taglen || asn1class != UNIVERSAL)) \ + return ASN1_BAD_ID; \ + if (tagnum == (tagexpect)) { \ + get_field_body(var, decoder); \ + } else var = optvalue; \ + } + /**** fields w/ length ****/ -#define get_lenfield_body(len,var,decoder)\ -retval = decoder(&subbuf,&(len),&(var));\ -if(retval) return retval;\ -if(!taglen && indef) { get_eoc(); }\ -next_tag() - -#define get_lenfield(len,var,tagexpect,decoder)\ -if(tagnum > (tagexpect)) return ASN1_MISSING_FIELD;\ -if(tagnum < (tagexpect)) return ASN1_MISPLACED_FIELD;\ -if((asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ - && (tagnum || taglen || asn1class != UNIVERSAL)) \ - return ASN1_BAD_ID;\ -get_lenfield_body(len,var,decoder) - -#define opt_lenfield(len,var,tagexpect,decoder)\ -if(tagnum == (tagexpect)){\ - get_lenfield_body(len,var,decoder); }\ -else { len = 0; var = 0; } - - -#define begin_structure()\ -asn1buf subbuf;\ -int seqindef;\ -int indef;\ -retval = asn1_get_sequence(buf,&length,&seqindef);\ -if(retval) return retval;\ -retval = asn1buf_imbed(&subbuf,buf,length,seqindef);\ -if(retval) return retval;\ -next_tag() - -#define end_structure()\ -retval = asn1buf_sync(buf,&subbuf,asn1class,tagnum,length,indef,seqindef);\ -if(retval) return retval -#define sequence_of(buf) \ -unsigned int length, taglen; \ -asn1_class asn1class; \ -asn1_construction construction; \ -asn1_tagnum tagnum; \ -int indef; \ -sequence_of_common(buf) - -#define sequence_of_common(buf) \ -int size=0; \ -asn1buf seqbuf; \ -int seqofindef; \ -retval = asn1_get_sequence(buf,&length,&seqofindef); \ -if(retval) return retval; \ -retval = asn1buf_imbed(&seqbuf,buf,length,seqofindef); \ -if(retval) return retval +/* similar to get_field_body */ +#define get_lenfield_body(len, var, decoder) \ + retval = decoder(&subbuf, &(len), &(var)); \ + if (retval) return retval; \ + if (!taglen && indef) { get_eoc(); } \ + next_tag() + +/* similar to get_field_body */ +#define get_lenfield(len, var, tagexpect, decoder) \ + if (tagnum > (tagexpect)) return ASN1_MISSING_FIELD; \ + if (tagnum < (tagexpect)) return ASN1_MISPLACED_FIELD; \ + if ((asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ + && (tagnum || taglen || asn1class != UNIVERSAL)) \ + return ASN1_BAD_ID; \ + get_lenfield_body(len, var, decoder) + +/* similar to opt_field */ +#define opt_lenfield(len, var, tagexpect, decoder) \ + if (tagnum == (tagexpect)) { \ + get_lenfield_body(len, var, decoder); \ + } else { len = 0; var = 0; } -#define sequence_of_no_tagvars(buf) \ -asn1_class eseqclass; \ -asn1_construction eseqconstr; \ -asn1_tagnum eseqnum; \ -unsigned int eseqlen; \ -int eseqindef; \ -sequence_of_common(buf) +/* + * begin_structure + * + * Declares some variables for decoding SEQUENCE types. This is meant + * to be called in an inner block that ends with a call to + * end_structure(). + */ +#define begin_structure() \ + asn1buf subbuf; \ + int seqindef; \ + int indef; \ + retval = asn1_get_sequence(buf, &length, &seqindef); \ + if (retval) return retval; \ + retval = asn1buf_imbed(&subbuf, buf, length, seqindef); \ + if (retval) return retval; \ + next_tag() + +/* skip trailing garbage */ +#define end_structure() \ + retval = asn1buf_sync(buf, &subbuf, asn1class, tagnum, \ + length, indef, seqindef); \ + if (retval) return retval -#define end_sequence_of_no_tagvars(buf) \ -retval = asn1_get_tag_indef(&seqbuf,&eseqclass,&eseqconstr, \ - &eseqnum,&eseqlen,&eseqindef); \ -if(retval) return retval; \ -retval = asn1buf_sync(buf,&seqbuf,eseqclass,eseqnum, \ - eseqlen,eseqindef,seqofindef); \ -if(retval) return retval; +/* + * sequence_of + * + * Declares some variables for decoding SEQUENCE OF types. This is + * meant to be called in an inner block that ends with a call to + * end_sequence_of(). + */ +#define sequence_of(buf) \ + unsigned int length, taglen; \ + asn1_class asn1class; \ + asn1_construction construction; \ + asn1_tagnum tagnum; \ + int indef; \ + sequence_of_common(buf) -#define end_sequence_of(buf) \ -retval = asn1_get_tag_indef(&seqbuf,&asn1class,&construction, \ - &tagnum,&taglen,&indef); \ -if(retval) return retval; \ -retval = asn1buf_sync(buf,&seqbuf,asn1class,tagnum, \ - length,indef,seqofindef); \ -if(retval) return retval; +/* + * sequence_of_no_tagvars + * + * This is meant for use inside decoder functions that have an outer + * sequence structure and thus declares variables of different names + * than does sequence_of() to avoid shadowing. + */ +#define sequence_of_no_tagvars(buf) \ + asn1_class eseqclass; \ + asn1_construction eseqconstr; \ + asn1_tagnum eseqnum; \ + unsigned int eseqlen; \ + int eseqindef; \ + sequence_of_common(buf) -#define cleanup()\ -return 0 +/* + * sequence_of_common + * + * Fetches the outer SEQUENCE OF length info into {length,seqofindef} + * and imbeds an inner buffer seqbuf. Unlike begin_structure(), it + * does not prefetch the next tag. + */ +#define sequence_of_common(buf) \ + int size = 0; \ + asn1buf seqbuf; \ + int seqofindef; \ + retval = asn1_get_sequence(buf, &length, &seqofindef); \ + if (retval) return retval; \ + retval = asn1buf_imbed(&seqbuf, buf, length, seqofindef); \ + if (retval) return retval +/* + * end_sequence_of + * + * Attempts to fetch an EOC tag, if any, and to sync over trailing + * garbage, if any. + */ +#define end_sequence_of(buf) \ + retval = asn1_get_tag_indef(&seqbuf, &asn1class, &construction, \ + &tagnum, &taglen, &indef); \ + if (retval) return retval; \ + retval = asn1buf_sync(buf, &seqbuf, asn1class, tagnum, \ + length, indef, seqofindef); \ + if (retval) return retval; +/* + * end_sequence_of_no_tagvars + * + * Like end_sequence_of(), but uses the different (non-shadowing) + * variable names. + */ +#define end_sequence_of_no_tagvars(buf) \ + retval = asn1_get_tag_indef(&seqbuf, &eseqclass, &eseqconstr, \ + &eseqnum, &eseqlen, &eseqindef); \ + if (retval) return retval; \ + retval = asn1buf_sync(buf, &seqbuf, eseqclass, eseqnum, \ + eseqlen, eseqindef, seqofindef); \ + if (retval) return retval; + +#define cleanup() \ + return 0 /* scalars */ asn1_error_code asn1_decode_kerberos_time(asn1buf *buf, krb5_timestamp *val) diff --git a/src/lib/krb5/asn.1/krb5_decode.c b/src/lib/krb5/asn.1/krb5_decode.c index 2073e2544..cb6e8f252 100644 --- a/src/lib/krb5/asn.1/krb5_decode.c +++ b/src/lib/krb5/asn.1/krb5_decode.c @@ -122,10 +122,14 @@ if(asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED)\ get_field_body(var,decoder) /* decode (or skip, if not present) an optional field */ -#define opt_field(var,tagexpect,decoder)\ -if(asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED)\ - clean_return(ASN1_BAD_ID);\ -if(tagnum == (tagexpect)){ get_field_body(var,decoder); } +#define opt_field(var,tagexpect,decoder) \ + if (asn1buf_remains(&subbuf, seqindef)) { \ + if (asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ + clean_return(ASN1_BAD_ID); \ + if (tagnum == (tagexpect)) { \ + get_field_body(var,decoder); \ + } \ + } /* field w/ accompanying length *********/ #define get_lenfield_body(len,var,decoder)\ @@ -143,13 +147,15 @@ if(asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED)\ get_lenfield_body(len,var,decoder) /* decode an optional field w/ length */ -#define opt_lenfield(len,var,tagexpect,decoder)\ -if(asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED)\ - clean_return(ASN1_BAD_ID);\ -if(tagnum == (tagexpect)){\ - get_lenfield_body(len,var,decoder);\ -} - +#define opt_lenfield(len,var,tagexpect,decoder) \ + if (asn1buf_remains(&subbuf, seqindef)) { \ + if (asn1class != CONTEXT_SPECIFIC || construction != CONSTRUCTED) \ + clean_return(ASN1_BAD_ID); \ + if (tagnum == (tagexpect)) { \ + get_lenfield_body(len,var,decoder); \ + } \ + } + /* clean up ******************************************************/ /* finish up */ -- 2.26.2