lib/libssh2
1
0
Fork 0
mirror of https://github.com/libssh2/libssh2.git synced 2026-10-24 03:48:58 +03:00
Code Activity

os400qc3: reject unbounded PKCS#1 keys

Browse Source 
It simply rejects unbounded PKCS#1 keys, that are deviant because they
should be DER-encoded. This allowed removal of a potential OOB read
culprit function `asn1_new_from_bytes`.

Also:
- included is a commit fixing a wrong EOC length bug in ASN.1 parser.
  Follow-up to 7391a24286 #2287

Reported-by: Syed Abdul Khaliq
Fixes #2665
Closes #2668
This commit is contained in:
Patrick Monnerat
2026-10-20 23:36:27 +02:00
committed by Viktor Szakats
parent ce6c52a530
commit 25e5a5ebed
1 changed files with 22 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/os400qc3.c
View File

@@ -574,7 +574,7 @@ static char *getASN1Element(struct asn1Element *elem, char *beg, char *end)
len = b;
else if(!(b &= 0x7F)) {
/* Unspecified length. Since we have all the data, we can determine the
* effective length by skipping element until an end element is
* effective length by skipping element until an EOC element is
* found.
*/
if(!elem->constructed)
@@ -585,10 +585,10 @@ static char *getASN1Element(struct asn1Element *elem, char *beg, char *end)
if(!beg)
return NULL;
}
if(beg >= end)
if(end - beg < 2 || beg[1]) /* EOC must have a zero length. */
return NULL;
elem->end = beg;
return beg + 1;
elem->end = beg; /* Do not include EOC as content. */
return beg + 2; /* After EOC element. */
}
else if(beg + b > end)
return NULL; /* Does not fit in source. */
@@ -652,23 +652,6 @@ static struct asn1Element *asn1_new(unsigned int type, unsigned int length)
return e;
}
static struct asn1Element *asn1_new_from_bytes(const unsigned char *data,
unsigned int length)
{
struct asn1Element *e;
struct asn1Element et;
if(!getASN1Element(&et, (unsigned char *)data,
(unsigned char *)data + length))
return NULL;
e = asn1_new(et.tag, et.end - et.beg);
if(e)
memcpy(e->header, data, e->end - e->header);
return e;
}
static void asn1delete(struct asn1Element *e)
{
if(e) {
@@ -890,7 +873,8 @@ static struct asn1Element *privatekeyinfo(struct asn1Element *privkey,
asn1delete(version);
asn1delete(algorithm);
asn1delete(privatekeyalgorithm);
if(!version || !algorithm || !privatekeyalgorithm) {
asn1delete(privatekey);
if(!version || !algorithm || !privatekeyalgorithm || !privatekey) {
asn1delete(privkeyinfo);
privkeyinfo = NULL;
}
@@ -2001,23 +1985,29 @@ static int pkcs1topkcs8(LIBSSH2_SESSION *session,
const unsigned char **data8, unsigned int *datalen8,
const unsigned char *data1, unsigned int datalen1)
{
struct asn1Element *prvk;
struct asn1Element prvk;
struct asn1Element *pkcs8;
unsigned char *data;
*data8 = NULL;
*datalen8 = 0;
if(datalen1 < 2)
/* Reject unbounded key element for the following reasons:
- supported by BER format, but not DER,
- not used for an RSA key in the real world (even OpenSSL rejects it),
- known to introduce vulnerabilities,
- signature/HMAC on it is likely to fail, due to the variant notation,
- adds an extra-level of container generation handling complexity.
*/
if(datalen1 < 2 || data1[1] == 0x80)
return -1;
prvk = asn1_new_from_bytes(data1, datalen1);
if(!prvk)
data = (unsigned char *)getASN1Element(&prvk, (char *)data1,
(char *)data1 + datalen1);
if(data != (unsigned char *)data1 + datalen1)
return -1;
pkcs8 = rsaprivatekeyinfo(prvk);
asn1delete(prvk);
if(!prvk) { /* FIXME: always false condition */
asn1delete(pkcs8);
pkcs8 = NULL;
}
pkcs8 = rsaprivatekeyinfo(&prvk);
if(!pkcs8)
return -1;
data = SSH2_ALLOC(session, pkcs8->end - pkcs8->header);