lynxis lazus has uploaded this change for review.
sim/class_tables: add a size attribute
osim_determine_apdu_case() doesn't know the size of the APDU,
this may result in reading invalid data outside the APDU.
Warning: this is an API breakage!
Instead adding a second function osim_determine_apdu_case_size()
could allow it. However because the class_tables.h also defines
the internal struct with the helper functions, which in turn has the same problem.
Change-Id: Iee50063399a0c3b29594e737f44aaa125fd06a2e
---
M include/osmocom/sim/class_tables.h
M src/sim/class_tables.c
M tests/sim/sim_test.c
3 files changed, 30 insertions(+), 16 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/78/43178/1
diff --git a/include/osmocom/sim/class_tables.h b/include/osmocom/sim/class_tables.h
index ec9ec49..2c7c206 100644
--- a/include/osmocom/sim/class_tables.h
+++ b/include/osmocom/sim/class_tables.h
@@ -20,7 +20,7 @@
struct osim_cla_ins_case {
uint8_t cla;
uint8_t cla_mask;
- int (*helper)(const struct osim_cla_ins_case *cic, const uint8_t *hdr);
+ int (*helper)(const struct osim_cla_ins_case *cic, const uint8_t *apdu, size_t apdu_size);
const uint8_t *ins_tbl;
};
@@ -36,4 +36,4 @@
extern const struct osim_cla_ins_card_profile osim_uicc_sim_cic_profile;
int osim_determine_apdu_case(const struct osim_cla_ins_card_profile *prof,
- const uint8_t *hdr);
+ const uint8_t *apdu, size_t apdu_size);
diff --git a/src/sim/class_tables.c b/src/sim/class_tables.c
index 7500fc5..0a792c6 100644
--- a/src/sim/class_tables.c
+++ b/src/sim/class_tables.c
@@ -16,6 +16,7 @@
*/
#include <stdint.h>
+#include <errno.h>
#include <osmocom/core/utils.h>
#include <osmocom/sim/class_tables.h>
@@ -139,13 +140,16 @@
};
static int uicc046_cla_ins_helper(const struct osim_cla_ins_case *cic,
- const uint8_t *hdr)
+ const uint8_t *hdr, size_t hdr_size)
{
- uint8_t ins = hdr[1];
- uint8_t p1 = hdr[2];
- uint8_t p2 = hdr[3];
- uint8_t p2_cmd;
+ uint8_t ins, p1, p2, p2_cmd;
+ if (hdr_size < 4)
+ return -EINVAL;
+
+ ins = hdr[1];
+ p1 = hdr[2];
+ p2 = hdr[3];
switch (ins) {
case 0x73: /* MANAGE SECURE CHANNEL */
if (p1 == 0x00) /* Retrieve UICC Endpoints */
@@ -182,12 +186,16 @@
}
static int gp_cla_ins_helper(const struct osim_cla_ins_case *cic,
- const uint8_t *hdr)
+ const uint8_t *hdr, size_t hdr_size)
{
- uint8_t ins = hdr[1];
- uint8_t p1 = hdr[2];
- uint8_t p3 = hdr[4];
+ uint8_t ins, p1, p3;
+ if (hdr_size < 5)
+ return -EINVAL;
+
+ ins = hdr[1];
+ p1 = hdr[2];
+ p3 = hdr[4];
switch (ins) {
case 0xE2: /* STORE DATA */
switch (p1 & 0x01) {
@@ -394,13 +402,17 @@
};
int osim_determine_apdu_case(const struct osim_cla_ins_card_profile *prof,
- const uint8_t *hdr)
+ const uint8_t *apdu, size_t apdu_len)
{
- uint8_t cla = hdr[0];
- uint8_t ins = hdr[1];
+ uint8_t cla, ins;
int i;
int rc;
+ if (apdu_len < 4)
+ return -EINVAL;
+
+ cla = apdu[0];
+ ins = apdu[1];
for (i = 0; i < prof->cic_arr_size; i++) {
const struct osim_cla_ins_case *cic = &prof->cic_arr[i];
if ((cla & cic->cla_mask) != cic->cla)
@@ -408,7 +420,8 @@
rc = cic->ins_tbl[ins];
switch (rc) {
case 0x80:
- return cic->helper(cic, hdr);
+ /* the length of the hdr is unknown */
+ return cic->helper(cic, apdu, apdu_len);
case 0x00:
/* continue with further cic, rather than abort
* now */
diff --git a/tests/sim/sim_test.c b/tests/sim/sim_test.c
index ab5d2be..801c559 100644
--- a/tests/sim/sim_test.c
+++ b/tests/sim/sim_test.c
@@ -19,6 +19,7 @@
#include <string.h>
#include <osmocom/sim/sim.h>
+#include <osmocom/core/utils.h>
#include <osmocom/sim/class_tables.h>
const uint8_t sim_sel_mf[] = { 0xA0, 0xA4, 0x00, 0x00, 0x02, 0x3F, 0x00 };
@@ -35,7 +36,7 @@
#define APDU_CASE_ASSERT(x, y) \
do { \
printf("Testing " #x "\n"); \
- int rc = osim_determine_apdu_case(&osim_uicc_sim_cic_profile, x); \
+ int rc = osim_determine_apdu_case(&osim_uicc_sim_cic_profile, x, ARRAY_SIZE(x)); \
if (rc != y) \
printf("%d (actual) != %d (intended)\n", rc, y); \
OSMO_ASSERT(rc == y); \
To view, visit change 43178. To unsubscribe, or for help writing mail filters, visit settings.