From 67890a738c0eb5e92c41189ba3c744fbc98a97ac Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Fri, 3 Dec 2021 13:40:20 +0100 Subject: [PATCH] OBJ_obj2txt(): fix off-by-one documentation of the result Also remove the outdated BUGS section and fix the coding style of the function. Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/17188) --- crypto/objects/obj_dat.c | 28 ++++++++++++++++------------ doc/man3/OBJ_nid2obj.pod | 28 +++++++++++----------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/crypto/objects/obj_dat.c b/crypto/objects/obj_dat.c index a146a96aad..eef80d63ce 100644 --- a/crypto/objects/obj_dat.c +++ b/crypto/objects/obj_dat.c @@ -314,7 +314,7 @@ ASN1_OBJECT *OBJ_nid2obj(int n) if (n == NID_undef) return NULL; if (n >= 0 && n < NUM_NID && nid_objs[n].nid != NID_undef) - return (ASN1_OBJECT *)&(nid_objs[n]); + return (ASN1_OBJECT *)&(nid_objs[n]); ad.type = ADDED_NID; ad.obj = &ob; @@ -410,8 +410,8 @@ ASN1_OBJECT *OBJ_txt2obj(const char *s, int no_name) int i, j; if (!no_name) { - if ((nid = OBJ_sn2nid(s)) != NID_undef || - (nid = OBJ_ln2nid(s)) != NID_undef) { + if ((nid = OBJ_sn2nid(s)) != NID_undef + || (nid = OBJ_ln2nid(s)) != NID_undef) { return OBJ_nid2obj(nid); } if (!ossl_isdigit(*s)) { @@ -485,17 +485,19 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name) use_bn = 0; for (;;) { unsigned char c = *p++; + len--; - if ((len == 0) && (c & 0x80)) + if (len == 0 && (c & 0x80) != 0) goto err; if (use_bn) { if (!BN_add_word(bl, c & 0x7f)) goto err; - } else + } else { l |= c & 0x7f; - if (!(c & 0x80)) + } + if ((c & 0x80) == 0) break; - if (!use_bn && (l > (ULONG_MAX >> 7L))) { + if (!use_bn && l > (ULONG_MAX >> 7L)) { if (bl == NULL && (bl = BN_new()) == NULL) goto err; if (!BN_set_word(bl, l)) @@ -505,8 +507,9 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name) if (use_bn) { if (!BN_lshift(bl, bl, 7)) goto err; - } else + } else { l <<= 7L; + } } if (first) { @@ -516,13 +519,14 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name) if (use_bn) { if (!BN_sub_word(bl, 80)) goto err; - } else + } else { l -= 80; + } } else { i = (int)(l / 40); l -= (long)(i * 40); } - if (buf && (buf_len > 1)) { + if (buf != NULL && buf_len > 1) { *buf++ = i + '0'; *buf = '\0'; buf_len--; @@ -536,7 +540,7 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name) if (!bndec) goto err; i = strlen(bndec); - if (buf) { + if (buf != NULL) { if (buf_len > 1) { *buf++ = '.'; *buf = '\0'; @@ -557,7 +561,7 @@ int OBJ_obj2txt(char *buf, int buf_len, const ASN1_OBJECT *a, int no_name) } else { BIO_snprintf(tbuf, sizeof(tbuf), ".%lu", l); i = strlen(tbuf); - if (buf && (buf_len > 0)) { + if (buf && buf_len > 0) { OPENSSL_strlcpy(buf, tbuf, buf_len); if (i > buf_len) { buf += buf_len; diff --git a/doc/man3/OBJ_nid2obj.pod b/doc/man3/OBJ_nid2obj.pod index ec12b842a5..748e76a584 100644 --- a/doc/man3/OBJ_nid2obj.pod +++ b/doc/man3/OBJ_nid2obj.pod @@ -71,12 +71,14 @@ as well as numerical forms. If I is 1 only the numerical form is acceptable. OBJ_obj2txt() converts the B I into a textual representation. -The representation is written as a null terminated string to I +Unless I is NULL, +the representation is written as a NUL-terminated string to I, where at most I bytes are written, truncating the result if necessary. -The total amount of space required is returned. If I is 0 then -if the object has a long or short name then that will be used, otherwise -the numerical form will be used. If I is 1 then the numerical -form will always be used. +In any case it returns the total string length, excluding the NUL character, +required for non-truncated representation, or -1 on error. +If I is 0 then if the object has a long or short name +then that will be used, otherwise the numerical form will be used. +If I is 1 then the numerical form will always be used. i2t_ASN1_OBJECT() is the same as OBJ_obj2txt() with the I set to zero. @@ -155,9 +157,10 @@ a NID or B on error. OBJ_add_sigid() returns 1 on success or 0 on error. -OBJ_obj2txt() returns the number of bytes written to I if I is big enough. -Otherwise, the result is truncated and the total amount of space required is returned. -It also returns -1 on error. +i2t_ASN1_OBJECT() an OBJ_obj2txt() return -1 on error. +On success, they return the length of the string written to I if I is +not NULL and I is big enough, otherwise the total string length. +Note that this does not count the trailing NUL character. =head1 EXAMPLES @@ -179,15 +182,6 @@ Create a new object directly: obj = OBJ_txt2obj("1.2.3.4", 1); -=head1 BUGS - -OBJ_obj2txt() is awkward and messy to use: it doesn't follow the -convention of other OpenSSL functions where the buffer can be set -to B to determine the amount of data that should be written. -Instead I must point to a valid buffer and I should -be set to a positive value. A buffer length of 80 should be more -than enough to handle any OID encountered in practice. - =head1 SEE ALSO L