ASN.1 code passes uninitialized values around
authorTom Yu <tlyu@mit.edu>
Tue, 8 Oct 2002 02:27:39 +0000 (02:27 +0000)
committerTom Yu <tlyu@mit.edu>
Tue, 8 Oct 2002 02:27:39 +0000 (02:27 +0000)
* 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
src/lib/krb5/asn.1/asn1_get.c
src/lib/krb5/asn.1/asn1_k_decode.c
src/lib/krb5/asn.1/krb5_decode.c

index 28ff9822a163af6296069a8776a03722985c90dc..db2fe2508f9ed02dccd5d437272795374b2bcf1c 100644 (file)
@@ -1,3 +1,20 @@
+2002-10-07  Tom Yu  <tlyu@mit.edu>
+
+       * 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  <raeburn@mit.edu>
 
        * asn1_decode.c, asn1_encode.c, asn1_get.c, asn1_get.h,
index 47dd2835b3ede24b8b383085bc8d9c1604a27ac6..e90ac6b06e3f7368a36e1f286efd5649decb76d7 100644 (file)
@@ -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;
index b1d47df6c981c7a5b97398b2346e528dd67c1d31..78d7e47258cf958f0898f454ed4f75fe0aaaa151 100644 (file)
 #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)
index 2073e25446606b31a0e6a3e6dde4ffea4db596cf..cb6e8f252ed7fccd965f0b72d81f104760aa98dc 100644 (file)
@@ -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 */