Mercurial > dropbear
diff keyimport.c @ 1316:2c9dac2d6707
merge 2016.74
author | Matt Johnston <matt@ucc.asn.au> |
---|---|
date | Thu, 21 Jul 2016 23:38:42 +0800 |
parents | 750ec4ec4cbe 8678e2cc1e53 |
children | 77c0d57a4410 |
line wrap: on
line diff
--- a/keyimport.c Sun Jun 19 20:38:38 2016 +0800 +++ b/keyimport.c Thu Jul 21 23:38:42 2016 +0800 @@ -62,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); @@ -243,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; @@ -259,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; } @@ -469,7 +477,7 @@ m_burn(buffer, sizeof(buffer)); return ret; - error: +error: m_burn(buffer, sizeof(buffer)); if (ret) { if (ret->keyblob) { @@ -584,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; } @@ -619,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; @@ -685,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; @@ -699,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; } @@ -708,7 +717,7 @@ &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; @@ -747,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; } @@ -756,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; @@ -1381,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; @@ -1409,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; @@ -1422,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) @@ -1483,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; @@ -1603,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; } @@ -1671,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 */ @@ -1695,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; } @@ -1908,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 */ +}