By design of d2i, it's the caller's responsibility to check a DER object
has been fully consumed. We read files from the disk, check hashes,
parse and validate the DER we encounter, but we do not make sure that
nothing follows the DER blob we parsed.
As Job noticed, it is possible to append data to a CRL and still have a
manifest display "Validation: OK" in file mode. This is partly possible
due to the fact that filemode has a rather lax notion of validity (since
it is an inspection tool), but also due to these missing checks.
The diff below checks for !=. Barring bugs in ASN1_item_d2i() (unheard
of!), only the < case should be possible, but it seems better to allow
for > as well. I guess we could assert <=.
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.101
diff -u -p -r1.101 cert.c
--- cert.c 30 Nov 2022 09:12:34 -0000 1.101
+++ cert.c 21 Feb 2023 01:48:00 -0000
@@ -641,13 +641,14 @@ cert_parse_ee_cert(const char *fn, X509
struct cert *
cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
{
- int extsz;
- int sia_present = 0;
- size_t i;
- X509 *x = NULL;
- X509_EXTENSION *ext = NULL;
- ASN1_OBJECT *obj;
- struct parse p;
+ const unsigned char *oder;
+ int extsz;
+ int sia_present = 0;
+ size_t i;
+ X509 *x = NULL;
+ X509_EXTENSION *ext = NULL;
+ ASN1_OBJECT *obj;
+ struct parse p;
/* just fail for empty buffers, the warning was printed elsewhere */
if (der == NULL)
@@ -658,8 +659,13 @@ cert_parse_pre(const char *fn, const uns
if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
err(1, NULL);
+ oder = der;
if ((x = d2i_X509(NULL, &der, len)) == NULL) {
cryptowarnx("%s: d2i_X509", p.fn);
+ goto out;
+ }
+ if (der != oder + len) {
+ warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
goto out;
}
Index: cms.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
retrieving revision 1.26
diff -u -p -r1.26 cms.c
--- cms.c 28 Dec 2022 21:30:18 -0000 1.26
+++ cms.c 21 Feb 2023 01:45:37 -0000
@@ -64,9 +64,10 @@ cms_extract_econtent(const char *fn, CMS
static int
cms_parse_validate_internal(X509 **xp, const char *fn, const unsigned char
*der,
- size_t derlen, const ASN1_OBJECT *oid, BIO *bio, unsigned char **res,
+ size_t len, const ASN1_OBJECT *oid, BIO *bio, unsigned char **res,
size_t *rsz)
{
+ const unsigned char *oder;
char buf[128], obuf[128];
const ASN1_OBJECT *obj, *octype;
ASN1_OCTET_STRING *kid = NULL;
@@ -89,8 +90,13 @@ cms_parse_validate_internal(X509 **xp, c
if (der == NULL)
return 0;
- if ((cms = d2i_CMS_ContentInfo(NULL, &der, derlen)) == NULL) {
+ oder = der;
+ if ((cms = d2i_CMS_ContentInfo(NULL, &der, len)) == NULL) {
cryptowarnx("%s: RFC 6488: failed CMS parse", fn);
+ goto out;
+ }
+ if (der != oder + len) {
+ warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
goto out;
}
Index: crl.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
retrieving revision 1.21
diff -u -p -r1.21 crl.c
--- crl.c 30 Nov 2022 09:03:44 -0000 1.21
+++ crl.c 21 Feb 2023 01:47:31 -0000
@@ -25,9 +25,10 @@
struct crl *
crl_parse(const char *fn, const unsigned char *der, size_t len)
{
- struct crl *crl;
- const ASN1_TIME *at;
- int rc = 0;
+ const unsigned char *oder;
+ struct crl *crl;
+ const ASN1_TIME *at;
+ int rc = 0;
/* just fail for empty buffers, the warning was printed elsewhere */
if (der == NULL)
@@ -36,8 +37,13 @@ crl_parse(const char *fn, const unsigned
if ((crl = calloc(1, sizeof(*crl))) == NULL)
err(1, NULL);
+ oder = der;
if ((crl->x509_crl = d2i_X509_CRL(NULL, &der, len)) == NULL) {
cryptowarnx("%s: d2i_X509_CRL", fn);
+ goto out;
+ }
+ if (der != oder + len) {
+ warnx("%s: %td bytes trailing garbage", fn, oder + len - der);
goto out;
}
