[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Some x509 branch review points

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Wed, 15 Oct 2014 11:15:42 +0100


In x509parse.c:x509_get_version:

  err = asn1_get_tag(p, end, &len,
                     ASN1_CONTEXT_SPECIFIC | ASN1_CONSTRUCTED | 0);

Why the "| 0"? It doesn't do anything.


In x509parse.c:x509_get_name:

  cur->next = apr_palloc(result_pool, sizeof(x509_name));

  if (cur->next == NULL)
    return SVN_NO_ERROR;

We generally assume apr_palloc will abort rather than return NULL,
that's certainly the assumption in other places in this file. If we
were to detect an allocation failure then returning no error is not


In x509parse.c:x509_get_name:

  oid = &cur->oid;
  oid->tag = **p;

  err = asn1_get_tag(p, end, &oid->len, ASN1_OID);
  if (err)
    return svn_error_create(SVN_ERR_X509_CERT_INVALID_NAME, err, NULL);

The asn1_get_tag() call verifies both that *p has not reached end and
that the tag is ASN1_OID. This happens after assigning **p to oid->tag,
so if asn1_get_tag were to detect that *p had reached end it would
happen after we had dereferenced **p. This bug occurs in other places:
x509_get_sig, x509_get_alg, etc.

Fix by assigning ASN1_OID explicitly or by moving the assignment after

Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Received on 2014-10-15 12:16:14 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.