r16306: Error handling in this asn1 code *sucks*. Fix a generic

class of memory leak bugs on error found by Klocwork (#123).
Many of these functions didn't free allocated memory on
error exit.
Jeremy.
(This used to be commit 8ef11a7c6de74024b7d535d959db2d462662a86f)

diff --git a/source3/libsmb/asn1.c b/source3/libsmb/asn1.c
index 8c986c9588fe26b94494d2b7286f92598a4c08a6..544ee78d406d3c209271f8a83896b465117d2c0e 100644 (file)
--- a/source3/libsmb/asn1.c
+++ b/source3/libsmb/asn1.c
@@ -393,20 +393,30 @@ BOOL asn1_check_OID(ASN1_DATA *data, const char *OID)
 BOOL asn1_read_GeneralString(ASN1_DATA *data, char **s)
 {
        int len;
-       if (!asn1_start_tag(data, ASN1_GENERAL_STRING)) return False;
+       char *str;
+
+       *s = NULL;
+
+       if (!asn1_start_tag(data, ASN1_GENERAL_STRING)) {
+               return False;
+       }
        len = asn1_tag_remaining(data);
        if (len < 0) {
                data->has_error = True;
                return False;
        }
-       *s = SMB_MALLOC(len+1);
-       if (! *s) {
+       str = SMB_MALLOC(len+1);
+       if (!str) {
                data->has_error = True;
                return False;
        }
-       asn1_read(data, *s, len);
-       (*s)[len] = 0;
+       asn1_read(data, str, len);
+       str[len] = 0;
        asn1_end_tag(data);
+
+       if (!data->has_error) {
+               *s = str;
+       }
        return !data->has_error;
 }
 

diff --git a/source3/libsmb/clispnego.c b/source3/libsmb/clispnego.c
index e87e9f0c7c9c2e3de1952a89694ec512cbf7a188..3dad37d9e1bc4d17b52d0c82e1e9f08a5d0978b6 100644 (file)
--- a/source3/libsmb/clispnego.c
+++ b/source3/libsmb/clispnego.c
@@ -163,11 +163,18 @@ BOOL spnego_parse_negTokenInit(DATA_BLOB blob,
        asn1_end_tag(&data);
 
        ret = !data.has_error;
+       if (data.has_error) {
+               int j;
+               SAFE_FREE(principal);
+               for(j = 0; j < i && j < ASN1_MAX_OIDS-1; j++) {
+                       SAFE_FREE(OIDs[j]);
+               }
+       }
+
        asn1_free(&data);
        return ret;
 }
 
-
 /*
   generate a negTokenTarg packet given a list of OIDs and a security blob
 */
@@ -212,7 +219,6 @@ DATA_BLOB gen_negTokenTarg(const char *OIDs[], DATA_BLOB blob)
        return ret;
 }
 
-
 /*
   parse a negTokenTarg packet giving a list of OIDs and a security blob
 */
@@ -248,6 +254,11 @@ BOOL parse_negTokenTarg(DATA_BLOB blob, char *OIDs[ASN1_MAX_OIDS], DATA_BLOB *se
        asn1_end_tag(&data);
 
        if (data.has_error) {
+               int j;
+               data_blob_free(secblob);
+               for(j = 0; j < i && j < ASN1_MAX_OIDS-1; j++) {
+                       SAFE_FREE(OIDs[j]);
+               }
                DEBUG(1,("Failed to parse negTokenTarg at offset %d\n", (int)data.ofs));
                asn1_free(&data);
                return False;
@@ -313,6 +324,10 @@ BOOL spnego_parse_krb5_wrap(DATA_BLOB blob, DATA_BLOB *ticket, uint8 tok_id[2])
 
        ret = !data.has_error;
 
+       if (data.has_error) {
+               data_blob_free(ticket);
+       }
+
        asn1_free(&data);
 
        return ret;
@@ -390,6 +405,12 @@ BOOL spnego_parse_challenge(const DATA_BLOB blob,
        asn1_end_tag(&data);
 
        ret = !data.has_error;
+
+       if (data.has_error) {
+               data_blob_free(chal1);
+               data_blob_free(chal2);
+       }
+
        asn1_free(&data);
        return ret;
 }
@@ -438,6 +459,7 @@ BOOL spnego_parse_auth(DATA_BLOB blob, DATA_BLOB *auth)
 
        if (data.has_error) {
                DEBUG(3,("spnego_parse_auth failed at %d\n", (int)data.ofs));
+               data_blob_free(auth);
                asn1_free(&data);
                return False;
        }
@@ -537,4 +559,3 @@ BOOL spnego_parse_auth_response(DATA_BLOB blob, NTSTATUS nt_status,
        asn1_free(&data);
        return True;
 }
-