Mercurial > dropbear
diff keyimport.c @ 1318:10e2a7727253 coverity
merge coverity
author | Matt Johnston <matt@ucc.asn.au> |
---|---|
date | Fri, 22 Jul 2016 00:08:02 +0800 |
parents | 2c9dac2d6707 |
children | 77c0d57a4410 |
line wrap: on
line diff
--- a/keyimport.c Fri Mar 18 22:47:33 2016 +0800 +++ b/keyimport.c Fri Jul 22 00:08:02 2016 +0800 @@ -36,9 +36,11 @@ #include "dbutil.h" #include "ecc.h" +#if DROPBEAR_ECDSA static const unsigned char OID_SEC256R1_BLOB[] = {0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07}; static const unsigned char OID_SEC384R1_BLOB[] = {0x2b, 0x81, 0x04, 0x00, 0x22}; static const unsigned char OID_SEC521R1_BLOB[] = {0x2b, 0x81, 0x04, 0x00, 0x23}; +#endif #define PUT_32BIT(cp, value) do { \ (cp)[3] = (unsigned char)(value); \ @@ -60,6 +62,8 @@ static int dropbear_write(const char*filename, sign_key * key); static sign_key *dropbear_read(const char* filename); +static int toint(unsigned u); + #if 0 static int sshcom_encrypted(const char *filename, char **comment); static struct ssh2_userkey *sshcom_read(const char *filename, char *passphrase); @@ -241,12 +245,11 @@ if ((*p & 0x1F) == 0x1F) { *id = 0; while (*p & 0x80) { - *id = (*id << 7) | (*p & 0x7F); p++, sourcelen--; if (sourcelen == 0) return -1; + *id = (*id << 7) | (*p & 0x7F); } - *id = (*id << 7) | (*p & 0x7F); p++, sourcelen--; } else { *id = *p & 0x1F; @@ -257,19 +260,26 @@ return -1; if (*p & 0x80) { + unsigned len; int n = *p & 0x7F; p++, sourcelen--; if (sourcelen < n) return -1; - *length = 0; + len = 0; while (n--) - *length = (*length << 8) | (*p++); + len = (len << 8) | (*p++); sourcelen -= n; + *length = toint(len); } else { *length = *p; p++, sourcelen--; } + if (*length < 0) { + printf("Negative ASN.1 length\n"); + return -1; + } + return p - (unsigned char *) source; } @@ -467,7 +477,7 @@ m_burn(buffer, sizeof(buffer)); return ret; - error: +error: m_burn(buffer, sizeof(buffer)); if (ret) { if (ret->keyblob) { @@ -582,8 +592,9 @@ /* Expect the SEQUENCE header. Take its absence as a failure to decrypt. */ ret = ber_read_id_len(p, key->keyblob_len, &id, &len, &flags); p += ret; - if (ret < 0 || id != 16) { - errmsg = "ASN.1 decoding failure - wrong password?"; + if (ret < 0 || id != 16 || len < 0 || + key->keyblob+key->keyblob_len-p < len) { + errmsg = "ASN.1 decoding failure"; goto error; } @@ -600,13 +611,13 @@ */ blobbuf = buf_new(3000); -#ifdef DROPBEAR_DSS +#if DROPBEAR_DSS if (key->type == OSSH_DSA) { buf_putstring(blobbuf, "ssh-dss", 7); retkey->type = DROPBEAR_SIGNKEY_DSS; } #endif -#ifdef DROPBEAR_RSA +#if DROPBEAR_RSA if (key->type == OSSH_RSA) { buf_putstring(blobbuf, "ssh-rsa", 7); retkey->type = DROPBEAR_SIGNKEY_RSA; @@ -617,7 +628,7 @@ ret = ber_read_id_len(p, key->keyblob+key->keyblob_len-p, &id, &len, &flags); p += ret; - if (ret < 0 || id != 2 || + if (ret < 0 || id != 2 || len < 0 || key->keyblob+key->keyblob_len-p < len) { errmsg = "ASN.1 decoding failure"; goto error; @@ -666,7 +677,7 @@ p += len; } -#ifdef DROPBEAR_ECDSA +#if DROPBEAR_ECDSA if (key->type == OSSH_EC) { unsigned char* private_key_bytes = NULL; int private_key_len = 0; @@ -683,7 +694,7 @@ &id, &len, &flags); p += ret; /* id==4 for octet string */ - if (ret < 0 || id != 4 || + if (ret < 0 || id != 4 || len < 0 || key->keyblob+key->keyblob_len-p < len) { errmsg = "ASN.1 decoding failure"; goto error; @@ -697,7 +708,7 @@ &id, &len, &flags); p += ret; /* id==0 */ - if (ret < 0 || id != 0) { + if (ret < 0 || id != 0 || len < 0) { errmsg = "ASN.1 decoding failure"; goto error; } @@ -706,28 +717,28 @@ &id, &len, &flags); p += ret; /* id==6 for object */ - if (ret < 0 || id != 6 || + if (ret < 0 || id != 6 || len < 0 || key->keyblob+key->keyblob_len-p < len) { errmsg = "ASN.1 decoding failure"; goto error; } if (0) {} -#ifdef DROPBEAR_ECC_256 +#if DROPBEAR_ECC_256 else if (len == sizeof(OID_SEC256R1_BLOB) && memcmp(p, OID_SEC256R1_BLOB, len) == 0) { retkey->type = DROPBEAR_SIGNKEY_ECDSA_NISTP256; curve = &ecc_curve_nistp256; } #endif -#ifdef DROPBEAR_ECC_384 +#if DROPBEAR_ECC_384 else if (len == sizeof(OID_SEC384R1_BLOB) && memcmp(p, OID_SEC384R1_BLOB, len) == 0) { retkey->type = DROPBEAR_SIGNKEY_ECDSA_NISTP384; curve = &ecc_curve_nistp384; } #endif -#ifdef DROPBEAR_ECC_521 +#if DROPBEAR_ECC_521 else if (len == sizeof(OID_SEC521R1_BLOB) && memcmp(p, OID_SEC521R1_BLOB, len) == 0) { retkey->type = DROPBEAR_SIGNKEY_ECDSA_NISTP521; @@ -745,7 +756,7 @@ &id, &len, &flags); p += ret; /* id==1 */ - if (ret < 0 || id != 1) { + if (ret < 0 || id != 1 || len < 0) { errmsg = "ASN.1 decoding failure"; goto error; } @@ -754,7 +765,7 @@ &id, &len, &flags); p += ret; /* id==3 for bit string */ - if (ret < 0 || id != 3 || + if (ret < 0 || id != 3 || len < 0 || key->keyblob+key->keyblob_len-p < len) { errmsg = "ASN.1 decoding failure"; goto error; @@ -830,15 +841,15 @@ int ret = 0; FILE *fp; -#ifdef DROPBEAR_RSA +#if DROPBEAR_RSA mp_int dmp1, dmq1, iqmp, tmpval; /* for rsa */ #endif if ( -#ifdef DROPBEAR_RSA +#if DROPBEAR_RSA key->type == DROPBEAR_SIGNKEY_RSA || #endif -#ifdef DROPBEAR_DSS +#if DROPBEAR_DSS key->type == DROPBEAR_SIGNKEY_DSS || #endif 0) @@ -1024,7 +1035,7 @@ } } /* end RSA and DSS handling */ -#ifdef DROPBEAR_ECDSA +#if DROPBEAR_ECDSA if (key->type == DROPBEAR_SIGNKEY_ECDSA_NISTP256 || key->type == DROPBEAR_SIGNKEY_ECDSA_NISTP384 || key->type == DROPBEAR_SIGNKEY_ECDSA_NISTP521) { @@ -1379,7 +1390,7 @@ memset(ret->keyblob, 0, ret->keyblob_size); m_free(ret->keyblob); } - memset(&ret, 0, sizeof(ret)); + memset(ret, 0, sizeof(*ret)); m_free(ret); } return NULL; @@ -1407,11 +1418,12 @@ pos = 8; if (key->keyblob_len < pos+4) goto done; /* key is far too short */ - pos += 4 + GET_32BIT(key->keyblob + pos); /* skip key type */ - if (key->keyblob_len < pos+4) + len = toint(GET_32BIT(key->keyblob + pos)); + if (len < 0 || len > key->keyblob_len - pos - 4) goto done; /* key is far too short */ - len = GET_32BIT(key->keyblob + pos); /* find cipher-type length */ - if (key->keyblob_len < pos+4+len) + pos += 4 + len; /* skip key type */ + len = toint(GET_32BIT(key->keyblob + pos)); /* find cipher-type length */ + if (len < 0 || len > key->keyblob_len - pos - 4) goto done; /* cipher type string is incomplete */ if (len != 4 || 0 != memcmp(key->keyblob + pos + 4, "none", 4)) answer = 1; @@ -1420,15 +1432,14 @@ *comment = dupstr(key->comment); memset(key->keyblob, 0, key->keyblob_size); m_free(key->keyblob); - memset(&key, 0, sizeof(key)); + memset(key, 0, sizeof(*key)); m_free(key); return answer; } static int sshcom_read_mpint(void *data, int len, struct mpint_pos *ret) { - int bits; - int bytes; + unsigned bits, bytes; unsigned char *d = (unsigned char *) data; if (len < 4) @@ -1481,7 +1492,7 @@ struct ssh2_userkey *ret = NULL, *retkey; const struct ssh_signkey *alg; unsigned char *blob = NULL; - int blobsize, publen, privlen; + int blobsize = 0, publen, privlen; if (!key) return NULL; @@ -1601,8 +1612,8 @@ /* * Strip away the containing string to get to the real meat. */ - len = GET_32BIT(ciphertext); - if (len > cipherlen-4) { + len = toint(GET_32BIT(ciphertext)); + if (len < 0 || len > cipherlen-4) { errmsg = "containing string was ill-formed"; goto error; } @@ -1669,7 +1680,8 @@ publen = pos; pos += put_mp(blob+pos, x.start, x.bytes); privlen = pos - publen; - } + } else + return NULL; dropbear_assert(privlen > 0); /* should have bombed by now if not */ @@ -1693,7 +1705,7 @@ } memset(key->keyblob, 0, key->keyblob_size); m_free(key->keyblob); - memset(&key, 0, sizeof(key)); + memset(key, 0, sizeof(*key)); m_free(key); return ret; } @@ -1906,3 +1918,27 @@ return ret; } #endif /* ssh.com stuff disabled */ + +/* From PuTTY misc.c */ +static int toint(unsigned u) +{ + /* + * Convert an unsigned to an int, without running into the + * undefined behaviour which happens by the strict C standard if + * the value overflows. You'd hope that sensible compilers would + * do the sensible thing in response to a cast, but actually I + * don't trust modern compilers not to do silly things like + * assuming that _obviously_ you wouldn't have caused an overflow + * and so they can elide an 'if (i < 0)' test immediately after + * the cast. + * + * Sensible compilers ought of course to optimise this entire + * function into 'just return the input value'! + */ + if (u <= (unsigned)INT_MAX) + return (int)u; + else if (u >= (unsigned)INT_MIN) /* wrap in cast _to_ unsigned is OK */ + return INT_MIN + (int)(u - (unsigned)INT_MIN); + else + return INT_MIN; /* fallback; should never occur on binary machines */ +}