laforge submitted this change.

View Change

Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved
host/cardem: fix integer overflow in process_do_rx_da()

osmo_apdu_segment_in() may return a negative number on receipt of
"unknown APDU case", and that would crash simtrace2-cardem-pcsc:

msgb(0x55d2cf7aa8a0): Not enough tailroom msgb_put
(allocated 920, head at 0, len 7, tailroom 1017 < want tailroom 65534)
backtrace() returned 19 addresses

Whenever osmo_apdu_segment_in() fails to recognize an APDU, the
communication is broken, because we don't know if we should continue
transmitting or receiving. Only a successful return value by would
allow us to know this. Do not crash, exit() gracefully.

Change-Id: I9e97b955a28ec886a429d744f9316e7e71be4481
Related: OS#5600
---
M host/src/simtrace2-cardem-pcsc.c
1 file changed, 7 insertions(+), 0 deletions(-)

diff --git a/host/src/simtrace2-cardem-pcsc.c b/host/src/simtrace2-cardem-pcsc.c
index 4820f36..374966d 100644
--- a/host/src/simtrace2-cardem-pcsc.c
+++ b/host/src/simtrace2-cardem-pcsc.c
@@ -167,6 +167,13 @@

rc = osmo_apdu_segment_in(&ac, data->data, data->data_len,
data->flags & CEMU_DATA_F_TPDU_HDR);
+ if (rc < 0) {
+ /* At this point the communication is broken. We cannot keep running, as we
+ * don't know if we should continue transmitting or receiving. Only a successful
+ * return value by osmo_apdu_segment_in() would allow us to know this. */
+ LOGCI(ci, LOGL_FATAL, "Failed to recognize APDU, terminating\n");
+ exit(1);
+ }

if (rc & APDU_ACT_TX_CAPDU_TO_CARD) {
struct msgb *tmsg = msgb_alloc(1024, "TPDU");

To view, visit change 28513. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-Change-Id: I9e97b955a28ec886a429d744f9316e7e71be4481
Gerrit-Change-Number: 28513
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge@osmocom.org>
Gerrit-MessageType: merged