Skip to content

TSSslServerCertUpdate() can crash or read out of bounds extracting a certificate CN #13484

Description

@maskit

In src/api/InkAPI.cc, TSSslServerCertUpdate() extracts the certificate common name like this:

const int              pos              = X509_NAME_get_index_by_NID(X509_get_subject_name(cert.get()), NID_commonName, -1);
const X509_NAME_ENTRY *common_name      = X509_NAME_get_entry(X509_get_subject_name(cert.get()), pos);
const ASN1_STRING     *common_name_asn1 = X509_NAME_ENTRY_get_data(common_name);
char *common_name_str = reinterpret_cast<char *>(const_cast<unsigned char *>(ASN1_STRING_get0_data(common_name_asn1)));
if (ASN1_STRING_length(common_name_asn1) != static_cast<int>(strlen(common_name_str))) {
  // Embedded null char
  return TS_ERROR;
}

Two problems:

  1. pos is -1 when the certificate has no CN. X509_NAME_get_entry(name, -1) returns nullptr, and neither X509_NAME_ENTRY_get_data(nullptr) nor the subsequent dereferences are guarded, so loading a certificate without a CN can null-deref.
  2. ASN1_STRING_get0_data() returns a length-prefixed buffer that is not guaranteed to be NUL-terminated. Calling strlen() on it can read past the end of the buffer if there is no NUL within its declared length. The existing length check only catches an embedded NUL (a CN shorter than the buffer), not the case where the buffer has no NUL at all.

This logic predates any in-flight OpenSSL 4 build-compatibility work (traces to commits from 2024) and is present on current master, independent of that work. It was noticed during review of #13476, which touches nearby lines for unrelated reasons but does not change this logic.

Suggested fix

Guard pos >= 0 and the common_name/common_name_asn1 pointers before use, and build the CN string from ASN1_STRING_get0_data()/ASN1_STRING_length() directly (e.g. std::string_view or std::string with an explicit length) instead of strlen().

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions