avoid calling sc_*_binary recursively

- lock the card early to avoid deselection of the file
- check on integer overflows of indices

fixes https://github.com/OpenSC/OpenSC/issues/1919
This commit is contained in:
Frank Morgner 2020-01-30 21:49:53 +01:00
parent a501c0d185
commit 14aaa64d3e
2 changed files with 97 additions and 112 deletions

View File

@ -628,7 +628,7 @@ int sc_read_binary(sc_card_t *card, unsigned int idx,
sc_log(card->ctx, "called; %"SC_FORMAT_LEN_SIZE_T"u bytes at index %d",
count, idx);
if (count == 0)
return 0;
LOG_FUNC_RETURN(card->ctx, SC_SUCCESS);
#ifdef ENABLE_SM
if (card->sm_ctx.ops.read_binary) {
@ -637,41 +637,39 @@ int sc_read_binary(sc_card_t *card, unsigned int idx,
LOG_FUNC_RETURN(card->ctx, r);
}
#endif
if (card->ops->read_binary == NULL)
LOG_FUNC_RETURN(card->ctx, SC_ERROR_NOT_SUPPORTED);
if (count > max_le) {
int bytes_read = 0;
unsigned char *p = buf;
/* lock the card now to avoid deselection of the file */
r = sc_lock(card);
LOG_TEST_RET(card->ctx, r, "sc_lock() failed");
r = sc_lock(card);
LOG_TEST_RET(card->ctx, r, "sc_lock() failed");
while (count > 0) {
size_t n = count > max_le ? max_le : count;
r = sc_read_binary(card, idx, p, n, flags);
if (r < 0) {
sc_unlock(card);
LOG_TEST_RET(card->ctx, r, "sc_read_binary() failed");
}
p += r;
if ((bytes_read > INT_MAX - r) || idx > UINT_MAX - r) {
/* `bytes_read + r` or `idx + r` would overflow */
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, SC_ERROR_OFFSET_TOO_LARGE);
}
idx += r;
bytes_read += r;
count -= r;
if (r == 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, bytes_read);
}
while (count > 0) {
size_t chunk = count > max_le ? max_le : count;
r = card->ops->read_binary(card, idx, buf, chunk, flags);
if (r == SC_SUCCESS) {
r = (int) chunk;
}
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, bytes_read);
if ((idx > SIZE_MAX - (size_t) r)
|| (size_t) r > count) {
/* `idx + r` or `count - r` would overflow */
r = SC_ERROR_OFFSET_TOO_LARGE;
}
if (r < 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, r);
}
count -= (size_t) r;
buf += (size_t) r;
idx += (size_t) r;
}
r = card->ops->read_binary(card, idx, buf, count, flags);
LOG_FUNC_RETURN(card->ctx, r);
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, SC_SUCCESS);
}
int sc_write_binary(sc_card_t *card, unsigned int idx,
@ -686,38 +684,40 @@ int sc_write_binary(sc_card_t *card, unsigned int idx,
sc_log(card->ctx, "called; %"SC_FORMAT_LEN_SIZE_T"u bytes at index %d",
count, idx);
if (count == 0)
LOG_FUNC_RETURN(card->ctx, 0);
LOG_FUNC_RETURN(card->ctx, SC_SUCCESS);
if (card->ops->write_binary == NULL)
LOG_FUNC_RETURN(card->ctx, SC_ERROR_NOT_SUPPORTED);
if (count > max_lc) {
int bytes_written = 0;
const u8 *p = buf;
/* lock the card now to avoid deselection of the file */
r = sc_lock(card);
LOG_TEST_RET(card->ctx, r, "sc_lock() failed");
r = sc_lock(card);
LOG_TEST_RET(card->ctx, r, "sc_lock() failed");
while (count > 0) {
size_t n = count > max_lc? max_lc : count;
r = sc_write_binary(card, idx, p, n, flags);
if (r < 0) {
sc_unlock(card);
LOG_TEST_RET(card->ctx, r, "sc_write_binary() failed");
}
p += r;
idx += r;
bytes_written += r;
count -= r;
if (r == 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, bytes_written);
}
while (count > 0) {
size_t chunk = count > max_lc ? max_lc : count;
r = card->ops->write_binary(card, idx, buf, chunk, flags);
if (r == SC_SUCCESS) {
r = (int) chunk;
}
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, bytes_written);
if ((idx > SIZE_MAX - (size_t) r)
|| (size_t) r > count) {
/* `idx + r` or `count - r` would overflow */
r = SC_ERROR_OFFSET_TOO_LARGE;
}
if (r < 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, r);
}
count -= (size_t) r;
buf += (size_t) r;
idx += (size_t) r;
}
r = card->ops->write_binary(card, idx, buf, count, flags);
LOG_FUNC_RETURN(card->ctx, r);
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, SC_SUCCESS);
}
int sc_update_binary(sc_card_t *card, unsigned int idx,
@ -732,7 +732,7 @@ int sc_update_binary(sc_card_t *card, unsigned int idx,
sc_log(card->ctx, "called; %"SC_FORMAT_LEN_SIZE_T"u bytes at index %d",
count, idx);
if (count == 0)
return 0;
LOG_FUNC_RETURN(card->ctx, SC_SUCCESS);
#ifdef ENABLE_SM
if (card->sm_ctx.ops.update_binary) {
@ -745,34 +745,35 @@ int sc_update_binary(sc_card_t *card, unsigned int idx,
if (card->ops->update_binary == NULL)
LOG_FUNC_RETURN(card->ctx, SC_ERROR_NOT_SUPPORTED);
if (count > max_lc) {
int bytes_written = 0;
const u8 *p = buf;
/* lock the card now to avoid deselection of the file */
r = sc_lock(card);
LOG_TEST_RET(card->ctx, r, "sc_lock() failed");
r = sc_lock(card);
LOG_TEST_RET(card->ctx, r, "sc_lock() failed");
while (count > 0) {
size_t n = count > max_lc? max_lc : count;
r = sc_update_binary(card, idx, p, n, flags);
if (r < 0) {
sc_unlock(card);
LOG_TEST_RET(card->ctx, r, "sc_update_binary() failed");
}
p += r;
idx += r;
bytes_written += r;
count -= r;
if (r == 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, bytes_written);
}
while (count > 0) {
size_t chunk = count > max_lc ? max_lc : count;
r = card->ops->update_binary(card, idx, buf, chunk, flags);
if (r == SC_SUCCESS) {
r = (int) chunk;
}
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, bytes_written);
if ((idx > SIZE_MAX - (size_t) r)
|| (size_t) r > count) {
/* `idx + r` or `count - r` would overflow */
r = SC_ERROR_OFFSET_TOO_LARGE;
}
if (r < 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, r);
}
count -= (size_t) r;
buf += (size_t) r;
idx += (size_t) r;
}
r = card->ops->update_binary(card, idx, buf, count, flags);
LOG_FUNC_RETURN(card->ctx, r);
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, SC_SUCCESS);
}
@ -881,7 +882,6 @@ int sc_put_data(sc_card_t *card, unsigned int tag, const u8 *buf, size_t len)
int sc_get_challenge(sc_card_t *card, u8 *rnd, size_t len)
{
int r;
size_t retry = 10;
if (len == 0)
return SC_SUCCESS;
@ -899,19 +899,17 @@ int sc_get_challenge(sc_card_t *card, u8 *rnd, size_t len)
if (r != SC_SUCCESS)
LOG_FUNC_RETURN(card->ctx, r);
while (len > 0 && retry > 0) {
while (len > 0) {
r = card->ops->get_challenge(card, rnd, len);
if (r == 0)
r = SC_ERROR_INVALID_DATA;
if (r < 0) {
sc_unlock(card);
LOG_FUNC_RETURN(card->ctx, r);
}
if (r > 0) {
rnd += (size_t) r;
len -= (size_t) r;
} else {
retry--;
}
rnd += (size_t) r;
len -= (size_t) r;
}
sc_unlock(card);

View File

@ -142,7 +142,7 @@ iso7816_read_binary(struct sc_card *card, unsigned int idx, u8 *buf, size_t coun
struct sc_apdu apdu;
int r;
if (idx > 0x7fff) {
if (idx > 0x7FFF) {
sc_log(ctx, "invalid EF offset: 0x%X > 0x7FFF", idx);
return SC_ERROR_OFFSET_TOO_LARGE;
}
@ -161,15 +161,6 @@ iso7816_read_binary(struct sc_card *card, unsigned int idx, u8 *buf, size_t coun
LOG_FUNC_RETURN(ctx, apdu.resplen);
LOG_TEST_RET(ctx, r, "Check SW error");
if (apdu.resplen < count) {
r = iso7816_read_binary(card, idx + apdu.resplen, buf + apdu.resplen, count - apdu.resplen, flags);
/* Ignore all but 'corrupted data' errors */
if (r == SC_ERROR_CORRUPTED_DATA)
LOG_FUNC_RETURN(ctx, SC_ERROR_CORRUPTED_DATA);
else if (r > 0)
apdu.resplen += r;
}
LOG_FUNC_RETURN(ctx, apdu.resplen);
}
@ -182,13 +173,12 @@ iso7816_read_record(struct sc_card *card,
int r;
sc_format_apdu(card, &apdu, SC_APDU_CASE_2, 0xB2, rec_nr, 0);
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
if (flags & SC_RECORD_BY_REC_NR)
apdu.p2 |= 0x04;
apdu.le = count;
apdu.resplen = count;
apdu.resp = buf;
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
if (flags & SC_RECORD_BY_REC_NR)
apdu.p2 |= 0x04;
fixup_transceive_length(card, &apdu);
r = sc_transmit_apdu(card, &apdu);
@ -208,13 +198,12 @@ iso7816_write_record(struct sc_card *card, unsigned int rec_nr,
int r;
sc_format_apdu(card, &apdu, SC_APDU_CASE_3, 0xD2, rec_nr, 0);
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
if (flags & SC_RECORD_BY_REC_NR)
apdu.p2 |= 0x04;
apdu.lc = count;
apdu.datalen = count;
apdu.data = buf;
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
if (flags & SC_RECORD_BY_REC_NR)
apdu.p2 |= 0x04;
fixup_transceive_length(card, &apdu);
r = sc_transmit_apdu(card, &apdu);
@ -234,11 +223,10 @@ iso7816_append_record(struct sc_card *card,
int r;
sc_format_apdu(card, &apdu, SC_APDU_CASE_3, 0xE2, 0, 0);
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
apdu.lc = count;
apdu.datalen = count;
apdu.data = buf;
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
fixup_transceive_length(card, &apdu);
r = sc_transmit_apdu(card, &apdu);
@ -258,13 +246,12 @@ iso7816_update_record(struct sc_card *card, unsigned int rec_nr,
int r;
sc_format_apdu(card, &apdu, SC_APDU_CASE_3, 0xDC, rec_nr, 0);
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
if (flags & SC_RECORD_BY_REC_NR)
apdu.p2 |= 0x04;
apdu.lc = count;
apdu.datalen = count;
apdu.data = buf;
apdu.p2 = (flags & SC_RECORD_EF_ID_MASK) << 3;
if (flags & SC_RECORD_BY_REC_NR)
apdu.p2 |= 0x04;
fixup_transceive_length(card, &apdu);
r = sc_transmit_apdu(card, &apdu);