Skip to content

Commit

Permalink
Avoid conditional malloc in parse_uri_attr()
Browse files Browse the repository at this point in the history 
Also reduce type casting, and get rid of (void *) casting.

Squash Coverity false positives CID 354021 and 354024.
  • Loading branch information
@mtrojnar
mtrojnar committed Mar 14, 2024
1 parent 1ab75f0 commit 15104c9
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 52 deletions.
18 changes: 9 additions & 9 deletions src/eng_back.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,15 @@ int ctx_finish(ENGINE_CTX *ctx)
static void *ctx_try_load_object(ENGINE_CTX *ctx,
const char *object_typestr,
void *(*match_func)(ENGINE_CTX *, PKCS11_TOKEN *,
const unsigned char *, size_t, const char *),
const char *, size_t, const char *),
const char *object_uri, const int login,
UI_METHOD *ui_method, void *callback_data)
{
PKCS11_SLOT *slot;
PKCS11_SLOT *found_slot = NULL, **matched_slots = NULL;
PKCS11_TOKEN *tok, *match_tok = NULL;
unsigned int n, m;
unsigned char *obj_id = NULL;
char *obj_id = NULL;
size_t obj_id_len = 0;
char *obj_label = NULL;
char tmp_pin[MAX_PIN_LENGTH+1];
Expand Down Expand Up @@ -428,7 +428,7 @@ static void *ctx_try_load_object(ENGINE_CTX *ctx,
}
if (obj_id_len != 0) {
ctx_log(ctx, 1, "id=");
dump_hex(ctx, 1, obj_id, obj_id_len);
dump_hex(ctx, 1, (unsigned char *)obj_id, obj_id_len);
}
if (obj_id_len != 0 && obj_label)
ctx_log(ctx, 1, " ");
Expand Down Expand Up @@ -589,7 +589,7 @@ static void *ctx_try_load_object(ENGINE_CTX *ctx,
static void *ctx_load_object(ENGINE_CTX *ctx,
const char *object_typestr,
void *(*match_func)(ENGINE_CTX *, PKCS11_TOKEN *,
const unsigned char *, size_t, const char *),
const char *, size_t, const char *),
const char *object_uri, UI_METHOD *ui_method, void *callback_data)
{
void *obj = NULL;
Expand Down Expand Up @@ -662,7 +662,7 @@ static PKCS11_CERT *cert_cmp(PKCS11_CERT *a, PKCS11_CERT *b)
}

static void *match_cert(ENGINE_CTX *ctx, PKCS11_TOKEN *tok,
const unsigned char *obj_id, size_t obj_id_len, const char *obj_label)
const char *obj_id, size_t obj_id_len, const char *obj_label)
{
PKCS11_CERT *certs, *selected_cert = NULL;
PKCS11_CERT cert_template = {0};
Expand Down Expand Up @@ -792,7 +792,7 @@ static int ctx_ctrl_load_cert(ENGINE_CTX *ctx, void *p)

static void *match_key(ENGINE_CTX *ctx, const char *key_type,
PKCS11_KEY *keys, unsigned int key_count,
const unsigned char *obj_id, size_t obj_id_len, const char *obj_label)
const char *obj_id, size_t obj_id_len, const char *obj_label)
{
PKCS11_KEY *selected_key = NULL;
unsigned int m;
Expand Down Expand Up @@ -849,7 +849,7 @@ static void *match_key(ENGINE_CTX *ctx, const char *key_type,
}

static void *match_key_int(ENGINE_CTX *ctx, PKCS11_TOKEN *tok,
const unsigned int isPrivate, const unsigned char *obj_id, size_t obj_id_len, const char *obj_label)
const unsigned int isPrivate, const char *obj_id, size_t obj_id_len, const char *obj_label)
{
PKCS11_KEY *keys;
PKCS11_KEY key_template = {0};
Expand Down Expand Up @@ -890,13 +890,13 @@ static void *match_key_int(ENGINE_CTX *ctx, PKCS11_TOKEN *tok,
}

static void *match_public_key(ENGINE_CTX *ctx, PKCS11_TOKEN *tok,
const unsigned char *obj_id, size_t obj_id_len, const char *obj_label)
const char *obj_id, size_t obj_id_len, const char *obj_label)
{
return match_key_int(ctx, tok, 0, obj_id, obj_id_len, obj_label);
}

static void *match_private_key(ENGINE_CTX *ctx, PKCS11_TOKEN *tok,
const unsigned char *obj_id, size_t obj_id_len, const char *obj_label)
const char *obj_id, size_t obj_id_len, const char *obj_label)
{
return match_key_int(ctx, tok, 1, obj_id, obj_id_len, obj_label);
}
Expand Down
85 changes: 44 additions & 41 deletions src/eng_parse.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#endif

static int hex_to_bin(ENGINE_CTX *ctx,
const char *in, unsigned char *out, size_t *outlen)
const char *in, char *out, size_t *outlen)
{
size_t left, count = 0;

Expand Down Expand Up @@ -75,7 +75,7 @@ static int hex_to_bin(ENGINE_CTX *ctx,
*outlen = 0;
return 0;
}
out[count++] = (unsigned char)byte;
out[count++] = byte;
left--;
}

Expand All @@ -86,7 +86,7 @@ static int hex_to_bin(ENGINE_CTX *ctx,
/* parse string containing slot and id information */
int parse_slot_id_string(ENGINE_CTX *ctx,
const char *slot_id, int *slot,
unsigned char *id, size_t *id_len, char **label)
char *id, size_t *id_len, char **label)
{
int n, i;

Expand Down Expand Up @@ -210,24 +210,13 @@ int parse_slot_id_string(ENGINE_CTX *ctx,
return 0;
}

static int parse_uri_attr(ENGINE_CTX *ctx,
const char *attr, int attrlen, unsigned char **field,
static int parse_uri_attr_len(ENGINE_CTX *ctx,
const char *attr, int attrlen, char *field,
size_t *field_len)
{
size_t max, outlen = 0;
unsigned char *out;
size_t max = *field_len, outlen = 0;
int ret = 1;

if (field_len) {
out = *field;
max = *field_len;
} else {
out = OPENSSL_malloc(attrlen + 1);
if (!out)
return 0;
max = attrlen + 1;
}

while (ret && attrlen && outlen < max) {
if (*attr == '%') {
if (attrlen < 3) {
Expand All @@ -239,34 +228,48 @@ static int parse_uri_attr(ENGINE_CTX *ctx,
tmp[0] = attr[1];
tmp[1] = attr[2];
tmp[2] = 0;
ret = hex_to_bin(ctx, tmp, &out[outlen++], &l);
ret = hex_to_bin(ctx, tmp, &field[outlen++], &l);
attrlen -= 3;
attr += 3;
}

} else {
out[outlen++] = *(attr++);
field[outlen++] = *(attr++);
attrlen--;
}
}
if (attrlen && outlen == max)
ret = 0;

if (ret)
*field_len = outlen;

return ret;
}

static int parse_uri_attr(ENGINE_CTX *ctx,
const char *attr, int attrlen, char **field)
{
int ret = 1;
size_t outlen = attrlen + 1;
char *out = OPENSSL_malloc(outlen);

if (!out)
return 0;

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

if (ret) {
if (field_len) {
*field_len = outlen;
} else {
out[outlen] = 0;
*field = out;
}
out[outlen] = 0;
*field = out;
} else {
if (!field_len)
OPENSSL_free(out);
OPENSSL_free(out);
}

return ret;
}


static int read_from_file(ENGINE_CTX *ctx,
const char *path, char *field, size_t *field_len)
{
Expand All @@ -293,24 +296,24 @@ static int read_from_file(ENGINE_CTX *ctx,
}

static int parse_pin_source(ENGINE_CTX *ctx,
const char *attr, int attrlen, unsigned char *field,
const char *attr, int attrlen, char *field,
size_t *field_len)
{
unsigned char *val;
char *val;
int ret = 1;

if (!parse_uri_attr(ctx, attr, attrlen, &val, NULL)) {
if (!parse_uri_attr(ctx, attr, attrlen, &val)) {
return 0;
}

if (!strncasecmp((const char *)val, "file:", 5)) {
ret = read_from_file(ctx, (const char *)(val + 5), (char *)field, field_len);
ret = read_from_file(ctx, (const char *)(val + 5), field, field_len);
} else if (*val == '|') {
ret = 0;
ctx_log(ctx, 0, "Unsupported pin-source syntax\n");
/* 'pin-source=/foo/bar' is commonly used */
} else {
ret = read_from_file(ctx, (const char *)val, (char *)field, field_len);
ret = read_from_file(ctx, (const char *)val, field, field_len);
}
OPENSSL_free(val);

Expand All @@ -319,7 +322,7 @@ static int parse_pin_source(ENGINE_CTX *ctx,

int parse_pkcs11_uri(ENGINE_CTX *ctx,
const char *uri, PKCS11_TOKEN **p_tok,
unsigned char *id, size_t *id_len, char *pin, size_t *pin_len,
char *id, size_t *id_len, char *pin, size_t *pin_len,
char **label)
{
PKCS11_TOKEN *tok;
Expand All @@ -344,30 +347,30 @@ int parse_pkcs11_uri(ENGINE_CTX *ctx,

if (!strncmp(p, "model=", 6)) {
p += 6;
rv = parse_uri_attr(ctx, p, end - p, (void *)&tok->model, NULL);
rv = parse_uri_attr(ctx, p, end - p, &tok->model);
} else if (!strncmp(p, "manufacturer=", 13)) {
p += 13;
rv = parse_uri_attr(ctx, p, end - p, (void *)&tok->manufacturer, NULL);
rv = parse_uri_attr(ctx, p, end - p, &tok->manufacturer);
} else if (!strncmp(p, "token=", 6)) {
p += 6;
rv = parse_uri_attr(ctx, p, end - p, (void *)&tok->label, NULL);
rv = parse_uri_attr(ctx, p, end - p, &tok->label);
} else if (!strncmp(p, "serial=", 7)) {
p += 7;
rv = parse_uri_attr(ctx, p, end - p, (void *)&tok->serialnr, NULL);
rv = parse_uri_attr(ctx, p, end - p, &tok->serialnr);
} else if (!strncmp(p, "object=", 7)) {
p += 7;
rv = parse_uri_attr(ctx, p, end - p, (void *)&newlabel, NULL);
rv = parse_uri_attr(ctx, p, end - p, &newlabel);
} else if (!strncmp(p, "id=", 3)) {
p += 3;
rv = parse_uri_attr(ctx, p, end - p, (void *)&id, id_len);
rv = parse_uri_attr_len(ctx, p, end - p, id, id_len);
id_set = 1;
} else if (!strncmp(p, "pin-value=", 10)) {
p += 10;
rv = pin_set ? 0 : parse_uri_attr(ctx, p, end - p, (void *)&pin, pin_len);
rv = pin_set ? 0 : parse_uri_attr_len(ctx, p, 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, end - p, (unsigned char *)pin, pin_len);
rv = pin_set ? 0 : parse_pin_source(ctx, p, 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
4 changes: 2 additions & 2 deletions src/engine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,12 @@ void ctx_log(ENGINE_CTX *ctx, int level, const char *format, ...)

int parse_pkcs11_uri(ENGINE_CTX *ctx,
const char *uri, PKCS11_TOKEN **p_tok,
unsigned char *id, size_t *id_len, char *pin, size_t *pin_len,
char *id, size_t *id_len, char *pin, size_t *pin_len,
char **label);

int parse_slot_id_string(ENGINE_CTX *ctx,
const char *slot_id, int *slot,
unsigned char *id, size_t * id_len, char **label);
char *id, size_t * id_len, char **label);

/* switch to legacy call if get0 variant is not available */
#ifndef HAVE_X509_GET0_NOTBEFORE
Expand Down

0 comments on commit 15104c9

Please sign in to comment.