Skip to content

Commit

Permalink
Add check for buffer size before writing certificate data
Browse files Browse the repository at this point in the history
  • Loading branch information
@aveenismail
aveenismail committed Dec 6, 2023
1 parent 9700b31 commit 8933ecf
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 25 deletions.
45 changes: 26 additions & 19 deletions lib/util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ static ykpiv_rc _write_metadata(ykpiv_state *state, uint8_t tag, uint8_t *data,
static ykpiv_rc _get_metadata_item(uint8_t *data, size_t cb_data, uint8_t tag, uint8_t **pp_item, size_t *pcb_item);
static ykpiv_rc _set_metadata_item(uint8_t *data, size_t *pcb_data, size_t cb_data_max, uint8_t tag, uint8_t *p_item, size_t cb_item);

static size_t _obj_size_max(ykpiv_state *state) {
size_t obj_size_max(ykpiv_state *state) {
return (state && state->model == DEVTYPE_NEOr3) ? CB_OBJ_MAX_NEO : CB_OBJ_MAX;
}

Expand Down Expand Up @@ -558,7 +558,7 @@ ykpiv_rc ykpiv_util_write_mscmap(ykpiv_state *state, ykpiv_container *containers
// calculate the required length of the encoded object
req_len = 1 /* data tag */ + (unsigned long)_ykpiv_set_length(buf, data_len) + data_len;

if (req_len > _obj_size_max(state)) {
if (req_len > obj_size_max(state)) {
res = YKPIV_SIZE_ERROR;
goto Cleanup;
}
Expand Down Expand Up @@ -600,7 +600,7 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat
*data_len = 0;

// allocate first page
cbData = _obj_size_max(state);
cbData = obj_size_max(state);
if (NULL == (pData = _ykpiv_alloc(state, cbData))) { res = YKPIV_MEMORY_ERROR; goto Cleanup; }

for (object_id = YKPIV_OBJ_MSROOTS1; object_id <= YKPIV_OBJ_MSROOTS5; object_id++) {
Expand Down Expand Up @@ -678,7 +678,7 @@ ykpiv_rc ykpiv_util_write_msroots(ykpiv_state *state, uint8_t *data, size_t data
size_t data_chunk = 0;
size_t n_objs = 0;
unsigned int i = 0;
size_t cb_obj_max = _obj_size_max(state);
size_t cb_obj_max = obj_size_max(state);

if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) return res;
if (YKPIV_OK != (res = _ykpiv_ensure_application_selected(state))) goto Cleanup;
Expand Down Expand Up @@ -1486,10 +1486,22 @@ uint32_t ykpiv_util_slot_object(uint8_t slot) {
return YKPIV_OK;
}

void ykpiv_util_write_certdata(uint8_t *rawdata, size_t rawdata_len, uint8_t compress_info, uint8_t* certdata, size_t *certdata_len) {
ykpiv_rc ykpiv_util_write_certdata(uint8_t *rawdata, size_t rawdata_len, uint8_t compress_info, uint8_t* certdata, size_t *certdata_len, size_t max_object_size) {
size_t offset = 0;
size_t buf_len = 0;

unsigned long len_bytes = get_length_size(rawdata_len);

// calculate the required length of the encoded object
buf_len = 1 /* cert tag */ + 3 /* compression tag + data*/ + 2 /* lrc */;
buf_len += get_length_size(rawdata_len);
buf_len += rawdata_len;

if (buf_len > *certdata_len || /* detect overflow of unsigned size_t */
buf_len > max_object_size) { /* obj_size_max includes limits for TLV encoding */
return YKPIV_SIZE_ERROR;
}

memmove(certdata + len_bytes + 1, rawdata, rawdata_len);

certdata[offset++] = TAG_CERT;
Expand All @@ -1501,6 +1513,7 @@ void ykpiv_util_write_certdata(uint8_t *rawdata, size_t rawdata_len, uint8_t com
certdata[offset++] = TAG_CERT_LRC;
certdata[offset++] = 0;
*certdata_len = offset;
return YKPIV_OK;
}

static ykpiv_rc _read_certificate(ykpiv_state *state, uint8_t slot, uint8_t *buf, size_t *buf_len) {
Expand All @@ -1526,9 +1539,9 @@ void ykpiv_util_write_certdata(uint8_t *rawdata, size_t rawdata_len, uint8_t com

static ykpiv_rc _write_certificate(ykpiv_state *state, uint8_t slot, uint8_t *data, size_t data_len, uint8_t certinfo) {
uint8_t buf[CB_OBJ_MAX] = {0};
size_t buf_len = sizeof buf;
int object_id = (int)ykpiv_util_slot_object(slot);
size_t offset = 0;
size_t req_len = 0;


if (-1 == object_id) return YKPIV_INVALID_OBJECT;

Expand All @@ -1545,19 +1558,13 @@ static ykpiv_rc _write_certificate(ykpiv_state *state, uint8_t slot, uint8_t *da
}

// encode certificate data for storage

// calculate the required length of the encoded object
req_len = 1 /* cert tag */ + 3 /* compression tag + data*/ + 2 /* lrc */;
req_len += _ykpiv_set_length(buf, data_len);
req_len += data_len;

if (req_len < data_len) return YKPIV_SIZE_ERROR; /* detect overflow of unsigned size_t */
if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR; /* obj_size_max includes limits for TLV encoding */

ykpiv_util_write_certdata(data, data_len, certinfo, buf, &offset);
ykpiv_rc res = YKPIV_OK;
if ( (res=ykpiv_util_write_certdata(data, data_len, certinfo, buf, &buf_len, obj_size_max(state))) != YKPIV_OK) {
return res;
}

// write onto device
return _ykpiv_save_object(state, object_id, buf, offset);
return _ykpiv_save_object(state, object_id, buf, buf_len);
}

/*
Expand Down Expand Up @@ -1804,7 +1811,7 @@ static ykpiv_rc _write_metadata(ykpiv_state *state, uint8_t tag, uint8_t *data,
uint8_t *pTemp = buf;
int obj_id = 0;

if (cb_data > (_obj_size_max(state) - CB_OBJ_TAG_MAX)) {
if (cb_data > (obj_size_max(state) - CB_OBJ_TAG_MAX)) {
return YKPIV_GENERIC_ERROR;
}

Expand Down
9 changes: 8 additions & 1 deletion lib/ykpiv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,14 @@ extern "C"
* @param certdata Constructed certificate data
* @param certdata_len Length of constructed certificate data
*/
void ykpiv_util_write_certdata(uint8_t *data, size_t data_len, uint8_t compress_info, uint8_t* certdata, size_t *certdata_len);
ykpiv_rc ykpiv_util_write_certdata(uint8_t *data, size_t data_len, uint8_t compress_info, uint8_t* certdata, size_t *certdata_len, size_t max_object_size);

/**
* Return the maximum object size depending on the YubiKey type
* @param state State handle
* @return Maximum object size
*/
size_t obj_size_max(ykpiv_state *state);

/**
* Write a certificate to a given slot
Expand Down
7 changes: 2 additions & 5 deletions ykcs11/token.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ CK_RV token_generate_key(ykpiv_state *state, gen_info_t *gen, CK_BYTE key, CK_BY
if(rv != CKR_OK)
return rv;

ykpiv_util_write_certdata(data, recv_len, YKPIV_CERTINFO_UNCOMPRESSED, certdata, &certdata_len);
ykpiv_util_write_certdata(data, recv_len, YKPIV_CERTINFO_UNCOMPRESSED, certdata, &certdata_len, obj_size_max(state));

if(*cert_len < (CK_ULONG)certdata_len) {
DBG("Certificate buffer too small.");
Expand Down Expand Up @@ -494,10 +494,7 @@ CK_RV token_import_cert(ykpiv_state *state, CK_ULONG cert_id, CK_BYTE_PTR in, CK
return rv;
}

if (cert_len > YKPIV_OBJ_MAX_SIZE)
return CKR_FUNCTION_FAILED;

ykpiv_util_write_certdata(in, cert_len, YKPIV_CERTINFO_UNCOMPRESSED, certdata,&certdata_len);
ykpiv_util_write_certdata(in, cert_len, YKPIV_CERTINFO_UNCOMPRESSED, certdata, &certdata_len, obj_size_max(state));

// Store the certificate into the token
if ((res = ykpiv_save_object(state, cert_id, certdata, certdata_len)) != YKPIV_OK)
Expand Down

0 comments on commit 8933ecf

Please sign in to comment.