The legacy 7bit conversion functions (those without the '_n_' in the name) gave wrong return values on 64 bit platforms due to unproper signed/unsigned conversions and the usage of SIZE_MAX. This patch fixes this by using a smaller max size (see GSM_7BIT_LEGACY_MAX_BUFFER_SIZE, currently set to 64k) for the legacy wrappers and by using unsigned int for max_septets. In addition, there are tests now that check the return values of legacy encoding and decoding. Sponsored-by: On-Waves ehf --- include/osmocom/gsm/gsm_utils.h | 3 +++ src/gsm/gsm_utils.c | 17 +++++++++++------ tests/sms/sms_test.c | 22 ++++++++++++++++++++++ tests/sms/sms_test.ok | 8 ++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/include/osmocom/gsm/gsm_utils.h b/include/osmocom/gsm/gsm_utils.h index f412e3e..02bfe4c 100644 --- a/include/osmocom/gsm/gsm_utils.h +++ b/include/osmocom/gsm/gsm_utils.h @@ -195,6 +195,9 @@ enum gsm_chan_t { }; /* Deprectated functions */ +/* Limit encoding and decoding to use no more than this amount of buffer bytes */ +#define GSM_7BIT_LEGACY_MAX_BUFFER_SIZE 0x10000 + int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length) OSMO_DEPRECATED("Use gsm_7bit_decode_n() instead"); int gsm_7bit_decode_ussd(char *decoded, const uint8_t *user_data, uint8_t length) OSMO_DEPRECATED("Use gsm_7bit_decode_n_ussd() instead"); int gsm_7bit_encode(uint8_t *result, const char *data) OSMO_DEPRECATED("Use gsm_7bit_encode_n() instead"); diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index 5241c91..198ec69 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -273,7 +273,7 @@ int gsm_7bit_encode_n(uint8_t *result, size_t n, const char *data, int *octets) { int y = 0; int o; - int max_septets = n * 8 / 7; + size_t max_septets = n * 8 / 7; /* prepare for the worst case, every character expanding to two bytes */ uint8_t *rdata = calloc(strlen(data) * 2, sizeof(uint8_t)); @@ -683,7 +683,8 @@ uint32_t gprs_tmsi2tlli(uint32_t p_tmsi, enum gprs_tlli_type type) int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t septet_l) { - gsm_7bit_decode_n(text, SIZE_MAX, user_data, septet_l); + gsm_7bit_decode_n(text, GSM_7BIT_LEGACY_MAX_BUFFER_SIZE, + user_data, septet_l); /* Mimic the original behaviour. */ return septet_l; @@ -691,21 +692,25 @@ int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t septet_l) int gsm_7bit_decode_ussd(char *text, const uint8_t *user_data, uint8_t length) { - return gsm_7bit_decode_n_ussd(text, SIZE_MAX, user_data, length); + return gsm_7bit_decode_n_ussd(text, GSM_7BIT_LEGACY_MAX_BUFFER_SIZE, + user_data, length); } int gsm_7bit_encode(uint8_t *result, const char *data) { int out; - return gsm_7bit_encode_n(result, SIZE_MAX, data, &out); + return gsm_7bit_encode_n(result, GSM_7BIT_LEGACY_MAX_BUFFER_SIZE, + data, &out); } int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets) { - return gsm_7bit_encode_n_ussd(result, SIZE_MAX, data, octets); + return gsm_7bit_encode_n_ussd(result, GSM_7BIT_LEGACY_MAX_BUFFER_SIZE, + data, octets); } int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) { - return gsm_7bit_encode_n(result, SIZE_MAX, data, octets); + return gsm_7bit_encode_n(result, GSM_7BIT_LEGACY_MAX_BUFFER_SIZE, + data, octets); } diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c index 2c9d8d8..755b321 100644 --- a/tests/sms/sms_test.c +++ b/tests/sms/sms_test.c @@ -280,6 +280,17 @@ int main(int argc, char** argv) /* test 7-bit encoding */ for (i = 0; i < ARRAY_SIZE(test_encode); ++i) { + /* Test legacy function (return value only) */ + septet_length = gsm_7bit_encode(coded, + (const char *) test_encode[i].input); + printf("Legacy encode case %d: " + "septet length %d (expected %d)\n" + , i + , septet_length, test_encode[i].expected_septet_length + ); + OSMO_ASSERT (septet_length == test_encode[i].expected_septet_length); + + /* Test new function */ memset(coded, 0x42, sizeof(coded)); septet_length = gsm_7bit_encode_n(coded, sizeof(coded), (const char *) test_encode[i].input, @@ -296,6 +307,7 @@ int main(int argc, char** argv) OSMO_ASSERT (octets_written == test_encode[i].expected_octet_length); OSMO_ASSERT (octets_written == computed_octet_length); OSMO_ASSERT (memcmp(coded, test_encode[i].expected, octets_written) == 0); + OSMO_ASSERT (septet_length == test_encode[i].expected_septet_length); /* check buffer limiting */ memset(coded, 0xaa, sizeof(coded)); @@ -357,6 +369,16 @@ int main(int argc, char** argv) /* test 7-bit decoding */ for (i = 0; i < ARRAY_SIZE(test_decode); ++i) { + /* Test legacy function (return value only) */ + if (!test_decode[i].ud_hdr_ind) { + nchars = gsm_7bit_decode(result, test_decode[i].input, + test_decode[i].expected_septet_length); + printf("Legacy decode case %d: " + "return value %d (expected %d)\n", + i, nchars, test_decode[i].expected_septet_length); + } + + /* Test new function */ memset(result, 0x42, sizeof(result)); nchars = gsm_7bit_decode_n_hdr(result, sizeof(result), test_decode[i].input, test_decode[i].expected_septet_length, test_decode[i].ud_hdr_ind); diff --git a/tests/sms/sms_test.ok b/tests/sms/sms_test.ok index a71567d..fa536ea 100644 --- a/tests/sms/sms_test.ok +++ b/tests/sms/sms_test.ok @@ -1,11 +1,19 @@ SMS testing +Legacy encode case 0: septet length 9 (expected 9) Encode case 0: Octet length 8 (expected 8, computed 8), septet length 9 (expected 9) +Legacy encode case 1: septet length 41 (expected 41) Encode case 1: Octet length 36 (expected 36, computed 36), septet length 41 (expected 41) +Legacy encode case 2: septet length 39 (expected 39) Encode case 2: Octet length 35 (expected 35, computed 35), septet length 39 (expected 39) +Legacy encode case 3: septet length 40 (expected 40) Encode case 3: Octet length 35 (expected 35, computed 35), septet length 40 (expected 40) +Legacy decode case 0: return value 9 (expected 9) Decode case 0: return value 9 (expected 9) +Legacy decode case 1: return value 41 (expected 41) Decode case 1: return value 40 (expected 40) +Legacy decode case 2: return value 39 (expected 39) Decode case 2: return value 31 (expected 31) +Legacy decode case 3: return value 40 (expected 40) Decode case 3: return value 32 (expected 32) Decode case 4: return value 153 (expected 153) Decode case 5: return value 40 (expected 40) -- 1.7.9.5