Skip to content

Address CodeQL findings in utility and example code #661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mtrojnar merged 4 commits into from
Jul 15, 2026
+67 −51
Merged

Address CodeQL findings in utility and example code #661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6425b3d
Fix URI attribute buffer termination
olszomal Jul 15, 2026
41b3d7b
Document intentional PKCS#11 module loading
olszomal Jul 15, 2026
9571819
Document certificate selection logic and simplify logging
olszomal Jul 15, 2026
c60d806
examples: Cleanse PKCS#11 PIN buffers after use
olszomal Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions examples/auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ int main(int argc, char *argv[])

unsigned char *random = NULL, *signature = NULL;

char password[20];
char password[20] = { 0 };
int rc, fd, logged_in;
unsigned int nslots, ncerts, siglen;

Expand Down Expand Up @@ -168,7 +168,7 @@ int main(int argc, char *argv[])

/* perform pkcs #11 login */
rc = PKCS11_login(slot, 0, password);
memset(password, 0, strlen(password));
OPENSSL_cleanse(password, sizeof(password));
if (rc != 0) {
fprintf(stderr, "PKCS11_login failed\n");
rc = 10;
Expand Down Expand Up @@ -285,6 +285,7 @@ int main(int argc, char *argv[])
rc = 0;

failed:
OPENSSL_cleanse(password, sizeof(password));
if (rc)
ERR_print_errors_fp(stderr);
if (random != NULL)
Expand Down
1 change: 1 addition & 0 deletions src/libpkcs11.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ C_LoadModule(const char *mspec, CK_FUNCTION_LIST_PTR_PTR funcs)
memset(mod, 0, sizeof(sc_pkcs11_module_t));
mod->_magic = MAGIC;

/* The caller intentionally selects the PKCS#11 module to load. */
#ifdef WIN32
mod->handle = LoadLibraryA(mspec);
#else
Expand Down
112 changes: 63 additions & 49 deletions src/util_uri.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,20 +695,25 @@ static int parse_uri_attr_len(UTIL_CTX *ctx,
return ret;
}

/*
* Decode a URI attribute into a newly allocated NUL-terminated string.
* URI decoding cannot increase the input length.
*/
static int parse_uri_attr(UTIL_CTX *ctx,
const char *attr, int attrlen, char **field)
const char *attr, size_t attrlen, char **field)
{
int ret = 1;
size_t outlen = attrlen + 1;
char *out = OPENSSL_malloc(outlen);
size_t outlen = attrlen;
char *out;

if (!out)
out = OPENSSL_malloc(outlen + 1); /* reserve 1 byte for NUL terminator */
if (out == NULL)
return 0;

ret = parse_uri_attr_len(ctx, attr, attrlen, out, &outlen);

if (ret) {
out[outlen] = 0;
out[outlen] = '\0';
*field = out;
} else {
OPENSSL_free(out);
Expand All @@ -717,7 +722,6 @@ static int parse_uri_attr(UTIL_CTX *ctx,
return ret;
}


static int read_from_file(UTIL_CTX *ctx,
const char *path, char *field, size_t *field_len)
{
Expand Down Expand Up @@ -752,7 +756,7 @@ static int read_from_file(UTIL_CTX *ctx,
}

static int parse_pin_source(UTIL_CTX *ctx,
const char *attr, int attrlen, char *field,
const char *attr, size_t attrlen, char *field,
size_t *field_len)
{
char *val;
Expand Down Expand Up @@ -803,30 +807,30 @@ static int parse_pkcs11_uri(UTIL_CTX *ctx,

if (!strncmp(p, "model=", 6)) {
p += 6;
rv = parse_uri_attr(ctx, p, (int)(end - p), &tok->model);
rv = parse_uri_attr(ctx, p, (size_t)(end - p), &tok->model);
} else if (!strncmp(p, "manufacturer=", 13)) {
p += 13;
rv = parse_uri_attr(ctx, p, (int)(end - p), &tok->manufacturer);
rv = parse_uri_attr(ctx, p, (size_t)(end - p), &tok->manufacturer);
} else if (!strncmp(p, "token=", 6)) {
p += 6;
rv = parse_uri_attr(ctx, p, (int)(end - p), &tok->label);
rv = parse_uri_attr(ctx, p, (size_t)(end - p), &tok->label);
} else if (!strncmp(p, "serial=", 7)) {
p += 7;
rv = parse_uri_attr(ctx, p, (int)(end - p), &tok->serialnr);
rv = parse_uri_attr(ctx, p, (size_t)(end - p), &tok->serialnr);
} else if (!strncmp(p, "object=", 7)) {
p += 7;
rv = parse_uri_attr(ctx, p, (int)(end - p), &newlabel);
rv = parse_uri_attr(ctx, p, (size_t)(end - p), &newlabel);
} else if (!strncmp(p, "id=", 3)) {
p += 3;
rv = parse_uri_attr_len(ctx, p, (int)(end - p), id, id_len);
rv = parse_uri_attr_len(ctx, p, (size_t)(end - p), id, id_len);
id_set = 1;
} else if (!strncmp(p, "pin-value=", 10)) {
p += 10;
rv = pin_set ? 0 : parse_uri_attr_len(ctx, p, (int)(end - p), pin, pin_len);
rv = pin_set ? 0 : parse_uri_attr_len(ctx, p, (size_t)(end - p), pin, pin_len);
pin_set = 1;
} else if (!strncmp(p, "pin-source=", 11)) {
p += 11;
rv = pin_set ? 0 : parse_pin_source(ctx, p, (int)(end - p), pin, pin_len);
rv = pin_set ? 0 : parse_pin_source(ctx, p, (size_t)(end - p), pin, pin_len);
pin_set = 1;
} else if (!strncmp(p, "type=", 5) || !strncmp(p, "object-type=", 12)) {
p = strchr(p, '=') + 1;
Expand Down Expand Up @@ -1250,15 +1254,49 @@ static PKCS11_CERT *cert_cmp(PKCS11_CERT *a, PKCS11_CERT *b)
}
}

static void log_cert(UTIL_CTX *ctx, unsigned int index,
PKCS11_CERT *cert, const char *which)
{
char *hexbuf;
char *expiry;

hexbuf = dump_hex((unsigned char *)cert->id, cert->id_len);
expiry = dump_expiry(cert);

if (which != NULL) {
UTIL_CTX_log(ctx, LOG_NOTICE,
"Returning %s certificate:%s%s%s%s%s%s\n", which,
hexbuf ? " id=" : "",
hexbuf ? hexbuf : "",
cert->label ? " label=" : "",
cert->label ? cert->label : "",
expiry ? " expiry=" : "",
expiry ? expiry : "");
} else {
UTIL_CTX_log(ctx, LOG_NOTICE,
" %2u %s%s%s%s%s%s\n", index,
hexbuf ? " id=" : "",
hexbuf ? hexbuf : "",
cert->label ? " label=" : "",
cert->label ? cert->label : "",
expiry ? " expiry=" : "",
expiry ? expiry : "");
}

OPENSSL_free(hexbuf);
OPENSSL_free(expiry);
}

/* Find and select a certificate matching the requested object ID or label. */
static void *match_cert(UTIL_CTX *ctx, PKCS11_TOKEN *tok,
const char *obj_id, size_t obj_id_len, const char *obj_label)
{
PKCS11_CERT *certs, *selected_cert = NULL;
PKCS11_CERT cert_template = {0};
unsigned int m, cert_count;
const char *which;
char *hexbuf, *expiry;

/* Build a certificate template from the requested label and ID. */
errno = 0;
cert_template.label = obj_label ? OPENSSL_strdup(obj_label) : NULL;
if (errno != 0) {
Expand All @@ -1275,6 +1313,7 @@ static void *match_cert(UTIL_CTX *ctx, PKCS11_TOKEN *tok,
cert_template.id_len = obj_id_len;
}

/* Enumerate certificates matching the template. */
if (PKCS11_enumerate_certs_ext(tok, &cert_template, &certs, &cert_count)) {
UTIL_CTX_log(ctx, LOG_ERR, "Unable to enumerate certificates\n");
goto cleanup;
Expand All @@ -1285,21 +1324,13 @@ static void *match_cert(UTIL_CTX *ctx, PKCS11_TOKEN *tok,
}
UTIL_CTX_log(ctx, LOG_NOTICE, "Found %u certificate%s:\n", cert_count, cert_count == 1 ? "" : "s");
if (obj_id_len != 0 || obj_label) {
/* Select the matching certificate with the longest validity
* period when the caller supplied an ID or label. */
which = "longest expiry matching";
for (m = 0; m < cert_count; m++) {
PKCS11_CERT *k = certs + m;

hexbuf = dump_hex((unsigned char *)k->id, k->id_len);
expiry = dump_expiry(k);
UTIL_CTX_log(ctx, LOG_NOTICE, " %2u %s%s%s%s%s%s\n", m + 1,
hexbuf ? " id=" : "",
hexbuf ? hexbuf : "",
k->label ? " label=" : "",
k->label ? k->label : "",
expiry ? " expiry=" : "",
expiry ? expiry : "");
OPENSSL_free(hexbuf);
OPENSSL_free(expiry);
log_cert(ctx, m + 1, k, NULL);

if (obj_label && obj_id_len != 0) {
if (k->label && strcmp(k->label, obj_label) == 0 &&
Expand All @@ -1319,21 +1350,13 @@ static void *match_cert(UTIL_CTX *ctx, PKCS11_TOKEN *tok,
}
}
} else {
/* Without explicit selection criteria, prefer the first certificate
* with a nonempty ID and otherwise use the first certificate. */
which = "first (with id present)";
for (m = 0; m < cert_count; m++) {
PKCS11_CERT *k = certs + m;

hexbuf = dump_hex((unsigned char *)k->id, k->id_len);
expiry = dump_expiry(k);
UTIL_CTX_log(ctx, LOG_NOTICE, " %2u %s%s%s%s%s%s\n", m + 1,
hexbuf ? " id=" : "",
hexbuf ? hexbuf : "",
k->label ? " label=" : "",
k->label ? k->label : "",
expiry ? " expiry=" : "",
expiry ? expiry : "");
OPENSSL_free(hexbuf);
OPENSSL_free(expiry);
log_cert(ctx, m + 1, k, NULL);

if (!selected_cert && k->id && *k->id) {
selected_cert = k; /* Use the first certificate with nonempty id */
Expand All @@ -1345,18 +1368,9 @@ static void *match_cert(UTIL_CTX *ctx, PKCS11_TOKEN *tok,
}
}

/* Log the certificate selected for return. */
if (selected_cert) {
hexbuf = dump_hex((unsigned char *)selected_cert->id, selected_cert->id_len);
expiry = dump_expiry(selected_cert);
UTIL_CTX_log(ctx, LOG_NOTICE, "Returning %s certificate:%s%s%s%s%s%s\n", which,
hexbuf ? " id=" : "",
hexbuf ? hexbuf : "",
selected_cert->label ? " label=" : "",
selected_cert->label ? selected_cert->label : "",
expiry ? " expiry=" : "",
expiry ? expiry : "");
OPENSSL_free(hexbuf);
OPENSSL_free(expiry);
log_cert(ctx, 0, selected_cert, which);
} else {
UTIL_CTX_log(ctx, LOG_ERR, "No matching certificate returned.\n");
}
Expand Down
Loading