[PATCH 1/2] sms: Added result buffer size parameter to 7bit conv funs
The 7bit<->8bit encoding/decoding functions didn't check whether there is still enough space in the destination buffer. Therefore a buffer size parameter ('n' like in snprintf) has been added to each of the functions which leads to truncation if the the buffer is too small. In addition, the return value of the decoding functions has been changed to number of characters written (excluding \0), so this value is always equal to strlen(decoded) if n>0. The old functions are still available as wrapper functions. --- include/osmocom/gsm/gsm_utils.h | 63 ++++++++++++++++-- src/gsm/gsm0480.c | 11 ++-- src/gsm/gsm_utils.c | 137 +++++++++++++++++++++++++-------------- src/gsm/libosmogsm.map | 7 +- tests/sms/sms_test.c | 128 ++++++++++++++++++++---------------- tests/sms/sms_test.ok | 10 +++ tests/ussd/ussd_test.c | 38 +++++++++-- 7 files changed, 268 insertions(+), 126 deletions(-) diff --git a/include/osmocom/gsm/gsm_utils.h b/include/osmocom/gsm/gsm_utils.h index 6cd46e4..bdb4c74 100644 --- a/include/osmocom/gsm/gsm_utils.h +++ b/include/osmocom/gsm/gsm_utils.h @@ -25,6 +25,7 @@ #ifndef GSM_UTILS_H #define GSM_UTILS_H +#include <stddef.h> #include <stdint.h> #define ADD_MODULO(sum, delta, modulo) do { \ @@ -56,17 +57,58 @@ enum gsm_band { const char *gsm_band_name(enum gsm_band band); enum gsm_band gsm_band_parse(const char *mhz); -int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length); -int gsm_7bit_decode_ussd(char *decoded, const uint8_t *user_data, uint8_t length); -int gsm_7bit_decode_hdr(char *decoded, const uint8_t *user_data, uint8_t length, uint8_t ud_hdr_ind); -int gsm_7bit_encode(uint8_t *result, const char *data); -int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets_written); -int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets_written); +/* + * The following functions write at most n octets to the destination buffer + * ('decoded' or 'result') and return the number of octets written (always + * <= n). + */ + +/*! + * \brief Decode a sequence of GSM 03.38 encoded 7 bit characters. + * + * \param decoded The destination buffer for the decoded characters. + * \param n A maximum of n chars is written (incl. \0). + * \param user_data A pointer to the start of the packed 7bit character + * sequence. + * \param length The length of the input sequence (in octets). + * + * \returns the number of (8 bit) chars written excluding the terminating \0. + */ +int gsm_7bit_decode_n(char *decoded, size_t n, const uint8_t *user_data, uint8_t length); + +/*! + * \brief Decode a sequence of 7 bit characters (USSD encoding). + * + * \see gsm_7bit_encode_n() + */ +int gsm_7bit_decode_n_ussd(char *decoded, size_t n, const uint8_t *user_data, uint8_t length); -/* the three functions below are helper functions and here for the unit test */ +/** + * \brief Encode a text string into GSM 03.38 encoded 7 bit characters. + * + * \param result The destination buffer for the packed 7 bit sequence. + * \param n A maximum of n octets is written. + * \param data A pointer to the start of the \0 terminated 8 bit character + * string. + * \param octets_written Iff not NULL, *octets_written will be set to the + * number of octets written to the result buffer. + * + * \returns the number of septets that have been created. + */ +int gsm_7bit_encode_n(uint8_t *result, size_t n, const char *data, int *octets_written); + +/*! + * \brief Encode a text string into GSM 03.38 encoded 7 bit characters (USSD encoding). + * + * \see gsm_7bit_decode_n() + */ +int gsm_7bit_encode_n_ussd(uint8_t *result, size_t n, const char *data, int *octets_written); + +/* the four functions below are helper functions and here for the unit test */ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len, uint8_t padding); int gsm_septet_encode(uint8_t *result, const char *data); uint8_t gsm_get_octet_len(const uint8_t sept_len); +int gsm_7bit_decode_n_hdr(char *decoded, size_t n, const uint8_t *user_data, uint8_t length, uint8_t ud_hdr_ind); unsigned int ms_class_gmsk_dbm(enum gsm_band band, int ms_class); @@ -154,5 +196,12 @@ enum gsm_chan_t { _GSM_LCHAN_MAX }; +/* Deprectated functions */ +int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length) __attribute__((deprecated ("Use gsm_7bit_decode_n() instead"))); +int gsm_7bit_decode_ussd(char *decoded, const uint8_t *user_data, uint8_t length) __attribute__((deprecated ("Use gsm_7bit_decode_n_ussd() instead"))); +int gsm_7bit_encode(uint8_t *result, const char *data) __attribute__((deprecated ("Use gsm_7bit_encode_n() instead"))); +int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets_written) __attribute__((deprecated ("Use gsm_7bit_encode_n_ussd() instead"))); +int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets_written) __attribute__((deprecated ("Use gsm_7bit_encode_n() instead"))); + #endif diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c index cc693fe..0f72593 100644 --- a/src/gsm/gsm0480.c +++ b/src/gsm/gsm0480.c @@ -105,7 +105,7 @@ struct msgb *gsm0480_create_unstructuredSS_Notify(int alertPattern, const char * msgb_put_u8(msg, ASN1_OCTET_STRING_TAG); ussd_len_ptr = msgb_put(msg, 1); data = msgb_put(msg, 0); - gsm_7bit_encode_ussd(data, text, &len); + gsm_7bit_encode_n_ussd(data, msgb_tailroom(msg), text, &len); msgb_put(msg, len); ussd_len_ptr[0] = len; /* USSD-String } */ @@ -172,7 +172,7 @@ struct msgb *gsm0480_create_notifySS(const char *text) msgb_put_u8(msg, 0x82); tmp_len = msgb_put(msg, 1); data = msgb_put(msg, 0); - gsm_7bit_encode_ussd(data, text, &len); + gsm_7bit_encode_n_ussd(data, msgb_tailroom(msg), text, &len); tmp_len[0] = len; msgb_put(msg, len); @@ -398,10 +398,7 @@ static int parse_process_uss_req(const uint8_t *uss_req_data, uint16_t length, if ((dcs == 0x0F) && (uss_req_data[5] == ASN1_OCTET_STRING_TAG)) { num_chars = (uss_req_data[6] * 8) / 7; - /* Prevent a mobile-originated buffer-overrun! */ - if (num_chars > MAX_LEN_USSD_STRING) - num_chars = MAX_LEN_USSD_STRING; - gsm_7bit_decode_ussd(req->text, + gsm_7bit_decode_n_ussd(req->text, sizeof (req->text), &(uss_req_data[7]), num_chars); rc = 1; } @@ -423,7 +420,7 @@ struct msgb *gsm0480_create_ussd_resp(uint8_t invoke_id, uint8_t trans_id, const /* First put the payload text into the message */ ptr8 = msgb_put(msg, 0); - gsm_7bit_encode_ussd(ptr8, text, &response_len); + gsm_7bit_encode_n_ussd(ptr8, msgb_tailroom(msg), text, &response_len); msgb_put(msg, response_len); /* Then wrap it as an Octet String */ diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index 3dd1537..d76a5f2 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -123,12 +123,18 @@ uint8_t gsm_get_octet_len(const uint8_t sept_len){ } /* GSM 03.38 6.2.1 Character unpacking */ -int gsm_7bit_decode_hdr(char *text, const uint8_t *user_data, uint8_t septet_l, uint8_t ud_hdr_ind) +int gsm_7bit_decode_n_hdr(char *text, size_t n, const uint8_t *user_data, uint8_t septet_l, uint8_t ud_hdr_ind) { int i = 0; int shift = 0; - uint8_t c; + uint8_t c7, c8; uint8_t next_is_ext = 0; + const char *text_buf_begin = text; + const char *text_buf_end = text + n; + int nchars; + + if (n == 0) + return 0; /* skip the user data header */ if (ud_hdr_ind) { @@ -139,50 +145,49 @@ int gsm_7bit_decode_hdr(char *text, const uint8_t *user_data, uint8_t septet_l, septet_l = septet_l - shift; } - for (i = 0; i < septet_l; i++) { - c = + for (i = 0; i < septet_l && text != text_buf_end - 1; i++) { + c7 = ((user_data[((i + shift) * 7 + 7) >> 3] << (7 - (((i + shift) * 7 + 7) & 7))) | (user_data[((i + shift) * 7) >> 3] >> (((i + shift) * 7) & 7))) & 0x7f; - /* this is an extension character */ if (next_is_ext) { + /* this is an extension character */ next_is_ext = 0; - *(text++) = gsm_7bit_alphabet[0x7f + c]; - continue; - } - - if (c == 0x1b && i + 1 < septet_l) { + c8 = gsm_7bit_alphabet[0x7f + c7]; + } else if (c7 == 0x1b && i + 1 < septet_l) { next_is_ext = 1; + continue; } else { - *(text++) = gsm_septet_lookup(c); + c8 = gsm_septet_lookup(c7); } + + *(text++) = c8; } - if (ud_hdr_ind) - i += shift; + nchars = (int)text - (int)text_buf_begin; + *text = '\0'; - return i; + return nchars; } -int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t septet_l) +int gsm_7bit_decode_n(char *text, size_t n, const uint8_t *user_data, uint8_t septet_l) { - return gsm_7bit_decode_hdr(text, user_data, septet_l, 0); + return gsm_7bit_decode_n_hdr(text, n, user_data, septet_l, 0); } -int gsm_7bit_decode_ussd(char *text, const uint8_t *user_data, uint8_t length) +int gsm_7bit_decode_n_ussd(char *text, size_t n, const uint8_t *user_data, uint8_t length) { - int i; + int nchars; - gsm_7bit_decode_hdr(text, user_data, length, 0); - i = strlen(text); + nchars = gsm_7bit_decode_n_hdr(text, n, user_data, length, 0); /* remove last <CR>, if it fits up to the end of last octet */ - if (i && (user_data[gsm_get_octet_len(length) - 1] >> 1) == '\r') - text[--i] = '\0'; + if (nchars && (user_data[gsm_get_octet_len(length) - 1] >> 1) == '\r') + text[--nchars] = '\0'; - return i; + return nchars; } /* GSM 03.38 6.2.1 Prepare character packing */ @@ -261,38 +266,23 @@ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len } /* GSM 03.38 6.2.1 Character packing */ -int gsm_7bit_encode(uint8_t *result, const char *data) -{ - int out; - return gsm_7bit_encode_oct(result, data, &out); -} - -int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets) -{ - int y; - - y = gsm_7bit_encode_oct(result, data, octets); - /* if last octet contains only one bit, add <CR> */ - if (((y * 7) & 7) == 1) - result[(*octets) - 1] |= ('\r' << 1); - /* if last character is <CR> and completely fills last octet, add - * another <CR>. */ - if (y && ((y * 7) & 7) == 0 && (result[(*octets) - 1] >> 1) == '\r') { - result[(*octets)++] = '\r'; - y++; - } - - return y; -} - -int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) +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; /* prepare for the worst case, every character expanding to two bytes */ uint8_t *rdata = calloc(strlen(data) * 2, sizeof(uint8_t)); y = gsm_septet_encode(rdata, data); - *octets = gsm_septets2octets(result, rdata, y, 0); + + if (y > max_septets) + y = max_septets; + + o = gsm_septets2octets(result, rdata, y, 0); + + if (octets) + *octets = o; free(rdata); @@ -309,6 +299,24 @@ int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) return y; } +int gsm_7bit_encode_n_ussd(uint8_t *result, size_t n, const char *data, int *octets) +{ + int y; + + y = gsm_7bit_encode_n(result, n, data, octets); + /* if last octet contains only one bit, add <CR> */ + if (((y * 7) & 7) == 1) + result[(*octets) - 1] |= ('\r' << 1); + /* if last character is <CR> and completely fills last octet, add + * another <CR>. */ + if (y && ((y * 7) & 7) == 0 && (result[(*octets) - 1] >> 1) == '\r' && *octets < n - 1) { + result[(*octets)++] = '\r'; + y++; + } + + return y; +} + /* convert power class to dBm according to GSM TS 05.05 */ unsigned int ms_class_gmsk_dbm(enum gsm_band band, int class) { @@ -662,3 +670,34 @@ uint32_t gprs_tmsi2tlli(uint32_t p_tmsi, enum gprs_tlli_type type) } return tlli; } + +/* Wrappers for deprecated functions: */ + +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); + + /* Mimic the original behaviour. */ + return 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); +} + +int gsm_7bit_encode(uint8_t *result, const char *data) +{ + int out; + return gsm_7bit_encode_n(result, SIZE_MAX, 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); +} + +int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) +{ + return gsm_7bit_encode_n(result, SIZE_MAX, data, octets); +} diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index 1b985e1..9d15d66 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -133,11 +133,16 @@ gsm48_rr_att_tlvdef; gsm_7bit_decode; gsm_7bit_decode_ussd; -gsm_7bit_decode_hdr; gsm_7bit_encode; gsm_7bit_encode_ussd; gsm_7bit_encode_oct; +gsm_7bit_decode_n; +gsm_7bit_decode_n_ussd; +gsm_7bit_decode_n_hdr; +gsm_7bit_encode_n; +gsm_7bit_encode_n_ussd; + gsm_arfcn2band; gsm_arfcn2freq10; gsm_freq102arfcn; diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c index e48f9a3..c0f5bf6 100644 --- a/tests/sms/sms_test.c +++ b/tests/sms/sms_test.c @@ -116,14 +116,14 @@ static const uint8_t concatenated_part2_enc[] = { static const struct test_case test_multiple_encode[] = { { - .input = concatenated_text, + .input = (const uint8_t *)concatenated_text, .expected = concatenated_part1_enc, .expected_octet_length = sizeof(concatenated_part1_enc), .expected_septet_length = concatenated_part1_septet_length, .ud_hdr_ind = 1, }, { - .input = concatenated_text, + .input = (const uint8_t *)concatenated_text, .expected = concatenated_part2_enc, .expected_octet_length = sizeof(concatenated_part2_enc), .expected_septet_length = concatenated_part2_septet_length, @@ -134,28 +134,28 @@ static const struct test_case test_multiple_encode[] = static const struct test_case test_encode[] = { { - .input = simple_text, + .input = (const uint8_t *)simple_text, .expected = simple_enc, .expected_octet_length = sizeof(simple_enc), .expected_septet_length = simple_septet_length, .ud_hdr_ind = 0, }, { - .input = escape_text, + .input = (const uint8_t *)escape_text, .expected = escape_enc, .expected_octet_length = sizeof(escape_enc), .expected_septet_length = escape_septet_length, .ud_hdr_ind = 0, }, { - .input = enhanced_text, + .input = (const uint8_t *)enhanced_text, .expected = enhanced_enc, .expected_octet_length = sizeof(enhanced_enc), .expected_septet_length = enhanced_septet_length, .ud_hdr_ind = 0, }, { - .input = enhancedV2_text, + .input = (const uint8_t *)enhancedV2_text, .expected = enhancedV2_enc, .expected_octet_length = sizeof(enhancedV2_enc), .expected_septet_length = enhancedV2_septet_length, @@ -168,42 +168,42 @@ static const struct test_case test_decode[] = { .input = simple_enc, .input_length = sizeof(simple_enc), - .expected = simple_text, + .expected = (const uint8_t *)simple_text, .expected_septet_length = simple_septet_length, .ud_hdr_ind = 0, }, { .input = escape_enc, .input_length = sizeof(escape_enc), - .expected = escape_text, + .expected = (const uint8_t *)escape_text, .expected_septet_length = escape_septet_length, .ud_hdr_ind = 0, }, { .input = enhanced_enc, .input_length = sizeof(enhanced_enc), - .expected = enhanced_text, + .expected = (const uint8_t *)enhanced_text, .expected_septet_length = enhanced_septet_length, .ud_hdr_ind = 0, }, { .input = enhancedV2_enc, .input_length = sizeof(enhancedV2_enc), - .expected = enhancedV2_text, + .expected = (const uint8_t *)enhancedV2_text, .expected_septet_length = enhancedV2_septet_length, .ud_hdr_ind = 0, }, { .input = concatenated_part1_enc, .input_length = sizeof(concatenated_part1_enc), - .expected = splitted_text_part1, + .expected = (const uint8_t *)splitted_text_part1, .expected_septet_length = concatenated_part1_septet_length_with_header, .ud_hdr_ind = 1, }, { .input = concatenated_part2_enc, .input_length = sizeof(concatenated_part2_enc), - .expected = splitted_text_part2, + .expected = (const uint8_t *)splitted_text_part2, .expected_septet_length = concatenated_part2_septet_length_with_header, .ud_hdr_ind = 1, }, @@ -216,7 +216,7 @@ static void test_octet_return() printf("Encoding some tests and printing number of septets/octets\n"); - septets = gsm_7bit_encode_oct((uint8_t *) out, "test1234", &oct); + septets = gsm_7bit_encode_n((uint8_t *) out, sizeof(out), "test1234", &oct); printf("SEPTETS: %d OCTETS: %d\n", septets, oct); printf("Done\n"); @@ -227,43 +227,58 @@ int main(int argc, char** argv) printf("SMS testing\n"); struct msgb *msg; uint8_t i; - + uint16_t buffer_size; uint8_t octet_length; + int octets_written; + uint8_t computed_octet_length; uint8_t septet_length; uint8_t gsm_septet_length; uint8_t coded[256]; uint8_t tmp[160]; uint8_t septet_data[256]; uint8_t ud_header[6]; + int nchars; char result[256]; /* test 7-bit encoding */ for (i = 0; i < ARRAY_SIZE(test_encode); ++i) { memset(coded, 0x42, sizeof(coded)); - septet_length = gsm_7bit_encode(coded, test_encode[i].input); - octet_length = gsm_get_octet_len(septet_length); - if (octet_length != test_encode[i].expected_octet_length) { - fprintf(stderr, "Encode case %d: Octet length failure. Got %d, expected %d\n", - i, octet_length, test_encode[i].expected_octet_length); - return -1; - } - - if (septet_length != test_encode[i].expected_septet_length){ - fprintf(stderr, "Encode case %d: Septet length failure. Got %d, expected %d\n", - i, septet_length, test_encode[i].expected_septet_length); - return -1; - } - - if (memcmp(coded, test_encode[i].expected, octet_length) != 0) { - fprintf(stderr, "Encoded content does not match for case %d\n", - i); - return -1; + septet_length = gsm_7bit_encode_n(coded, sizeof(coded), + (const char *)test_encode[i].input, + &octets_written); + computed_octet_length = gsm_get_octet_len(septet_length); + printf("Encode case %d: " + "Octet length %d (expected %d, computed %d), " + "septet length %d (expected %d)\n" + , i + , octets_written, test_encode[i].expected_octet_length, computed_octet_length + , septet_length, test_encode[i].expected_septet_length + ); + + 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); + + /* check buffer limiting */ + memset(coded, 0xaa, sizeof(coded)); + + for (buffer_size = 0; + buffer_size < test_encode[i].expected_octet_length + 1 + && buffer_size < sizeof(coded) - 1; + ++buffer_size) + { + gsm_7bit_encode_n(coded, buffer_size, + (const char *)test_encode[i].input, + &octets_written); + + OSMO_ASSERT(octets_written <= buffer_size); + OSMO_ASSERT(coded[buffer_size] == 0xaa); } } /* Test: encode multiple SMS */ - int number_of_septets = gsm_septet_encode(septet_data, test_multiple_encode[0].input); + int number_of_septets = gsm_septet_encode(septet_data, (const char *)test_multiple_encode[0].input); /* SMS part 1 */ memset(tmp, 0x42, sizeof(tmp)); @@ -281,11 +296,7 @@ int main(int argc, char** argv) memset(coded, 0x42, sizeof(coded)); memcpy(coded, tmp, octet_length + 6); - if (memcmp(coded, test_multiple_encode[0].expected, octet_length) != 0) { - fprintf(stderr, "Multiple-SMS encoded content does not match for part 1\n"); - return -1; - } - + OSMO_ASSERT(memcmp(coded, test_multiple_encode[0].expected, octet_length) == 0); /* SMS part 2 */ memset(tmp, 0x42, sizeof(tmp)); @@ -303,27 +314,34 @@ int main(int argc, char** argv) memset(coded, 0x42, sizeof(coded)); memcpy(coded, tmp, octet_length + 6); - if (memcmp(coded, test_multiple_encode[1].expected, octet_length) != 0) { - fprintf(stderr, "Multiple-SMS encoded content does not match for part 2\n"); - return -1; - } - - + OSMO_ASSERT(memcmp(coded, test_multiple_encode[1].expected, octet_length) == 0); /* test 7-bit decoding */ for (i = 0; i < ARRAY_SIZE(test_decode); ++i) { - memset(result, 0x42, sizeof(coded)); - septet_length = gsm_7bit_decode_hdr(result, test_decode[i].input, + 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); - - if (strcmp(result, test_decode[i].expected) != 0) { - fprintf(stderr, "Test case %d failed to decode.\n", i); - return -1; - } - if (septet_length != test_decode[i].expected_septet_length) { - fprintf(stderr, "Decode case %d: Septet length failure. Got %d, expected %d\n", - i, septet_length, test_decode[i].expected_septet_length); - return -1; + printf("Decode case %d: return value %d (expected %d)\n", i, nchars, strlen(result)); + + OSMO_ASSERT(strcmp(result, (const char *)test_decode[i].expected) == 0); + OSMO_ASSERT(nchars == strlen(result)); + + /* check buffer limiting */ + memset(result, 0xaa, sizeof(result)); + + for (buffer_size = 0; + buffer_size < test_encode[i].expected_septet_length + 1 + && buffer_size < sizeof(result) - 1; + ++buffer_size) + { + nchars = gsm_7bit_decode_n_hdr(result, buffer_size, test_decode[i].input, + test_decode[i].expected_septet_length, test_decode[i].ud_hdr_ind); + + OSMO_ASSERT(nchars <= buffer_size); + OSMO_ASSERT(result[buffer_size] == (char)0xaa); + if (buffer_size > 0) { + OSMO_ASSERT(result[nchars] == '\0'); + } } } diff --git a/tests/sms/sms_test.ok b/tests/sms/sms_test.ok index ce6cb17..915a59c 100644 --- a/tests/sms/sms_test.ok +++ b/tests/sms/sms_test.ok @@ -1,4 +1,14 @@ SMS testing +Encode case 0: Octet length 8 (expected 8, computed 8), septet length 9 (expected 9) +Encode case 1: Octet length 36 (expected 36, computed 36), septet length 41 (expected 41) +Encode case 2: Octet length 35 (expected 35, computed 35), septet length 39 (expected 39) +Encode case 3: Octet length 35 (expected 35, computed 35), septet length 40 (expected 40) +Decode case 0: return value 9 (expected 9) +Decode case 1: return value 40 (expected 40) +Decode case 2: return value 31 (expected 31) +Decode case 3: return value 32 (expected 32) +Decode case 4: return value 153 (expected 153) +Decode case 5: return value 40 (expected 40) Encoding some tests and printing number of septets/octets SEPTETS: 8 OCTETS: 7 Done diff --git a/tests/ussd/ussd_test.c b/tests/ussd/ussd_test.c index e3e8e08..9985ac5 100644 --- a/tests/ussd/ussd_test.c +++ b/tests/ussd/ussd_test.c @@ -73,20 +73,44 @@ static void test_7bit_ussd(const char *text, const char *encoded_hex, const char { uint8_t coded[256]; char decoded[256]; - int y; + int octets_written; + int buffer_size; + int nchars; printf("original = %s\n", osmo_hexdump((uint8_t *)text, strlen(text))); - gsm_7bit_encode_ussd(coded, text, &y); - printf("encoded = %s\n", osmo_hexdump(coded, y)); + gsm_7bit_encode_n_ussd(coded, sizeof(coded), text, &octets_written); + printf("encoded = %s\n", osmo_hexdump(coded, octets_written)); - OSMO_ASSERT(strcmp(encoded_hex, osmo_hexdump_nospc(coded, y)) == 0); + OSMO_ASSERT(strcmp(encoded_hex, osmo_hexdump_nospc(coded, octets_written)) == 0); - gsm_7bit_decode_ussd(decoded, coded, y * 8 / 7); - y = strlen(decoded); - printf("decoded = %s\n\n", osmo_hexdump((uint8_t *)decoded, y)); + gsm_7bit_decode_n_ussd(decoded, sizeof(decoded), coded, octets_written * 8 / 7); + octets_written = strlen(decoded); + printf("decoded = %s\n\n", osmo_hexdump((uint8_t *)decoded, octets_written)); OSMO_ASSERT(strncmp(text, decoded, strlen(text)) == 0); OSMO_ASSERT(strcmp(appended_after_decode, decoded + strlen(text)) == 0); + + /* check buffer limiting */ + memset(decoded, 0xaa, sizeof(decoded)); + + for (buffer_size = 0; buffer_size < sizeof(decoded) - 1; ++buffer_size) + { + nchars = gsm_7bit_decode_n_ussd(decoded, buffer_size, coded, octets_written * 8 / 7); + OSMO_ASSERT(nchars <= buffer_size); + OSMO_ASSERT(decoded[buffer_size] == (char)0xaa); + if (buffer_size > 0) { + OSMO_ASSERT(decoded[nchars] == '\0'); + } + } + + memset(coded, 0xaa, sizeof(coded)); + + for (buffer_size = 0; buffer_size < sizeof(coded) - 1; ++buffer_size) + { + gsm_7bit_encode_n_ussd(coded, buffer_size, text, &octets_written); + OSMO_ASSERT(octets_written <= buffer_size); + OSMO_ASSERT(coded[buffer_size] == 0xaa); + } } int main(int argc, char **argv) -- 1.7.9.5
The table structure is not intuitive, so this comment shall aid to understand this. --- src/gsm/gsm_utils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index d76a5f2..5de4c4b 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -80,8 +80,12 @@ * left out as they can't be handled with a char and * since most phones don't display or write these * characters this would only needlessly make the code - * more complex -*/ + * more complex. + * + * Note that this table contains the latin1->7bit mapping _and_ has + * been merged with the reverse mapping (7bit->latin1) for the + * extended characters at offset 0x7f. + */ static unsigned char gsm_7bit_alphabet[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x0a, 0xff, 0xff, 0x0d, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, -- 1.7.9.5
On Mon, Aug 12, 2013 at 02:08:17PM +0200, Jacob Erlbeck wrote:
The 7bit<->8bit encoding/decoding functions didn't check whether there is still enough space in the destination buffer. Therefore a buffer size parameter ('n' like in snprintf) has been added to each of the functions which leads to truncation if the the buffer is too small.
I think there has to be a comma and 'the the'
In addition, the return value of the decoding functions has been changed to number of characters written (excluding \0), so this value is always equal to strlen(decoded) if n>0.
I commented that in the code, but maybe here as well. Maybe it is easier to forbid n==0 and add an OSMO_ASSERT? The argument for that is one can unconditionally printf the result?
+/* + * The following functions write at most n octets to the destination buffer + * ('decoded' or 'result') and return the number of octets written (always + * <= n). + */
Doxygen has the concept of groups[1], do you want to use that for these methods or do you just want to remove this comment?
msgb_put_u8(msg, ASN1_OCTET_STRING_TAG); ussd_len_ptr = msgb_put(msg, 1); data = msgb_put(msg, 0); - gsm_7bit_encode_ussd(data, text, &len); + gsm_7bit_encode_n_ussd(data, msgb_tailroom(msg), text, &len);
i think there is no test coverage for all of these calls.
+ gsm_7bit_decode_n_ussd(req->text, sizeof (req->text),
no space. :) or just forbid this case? OSMO_ASSERT(n > 0)? This way no one can accidently do a printf on the text and crash?
- if (c == 0x1b && i + 1 < septet_l) { + c8 = gsm_7bit_alphabet[0x7f + c7]; + } else if (c7 == 0x1b && i + 1 < septet_l) { next_is_ext = 1; + continue;
we try to avoid continue/break inside if statements. I have no counter proposal for the code flow though. :}
+ nchars = (int)text - (int)text_buf_begin;
I thought you wanted to use ptrdiff_t here? I think we should not truncate a 64bit point to potentially 32bit.
-int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) +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;
/* prepare for the worst case, every character expanding to two bytes */ uint8_t *rdata = calloc(strlen(data) * 2, sizeof(uint8_t));
while we are here we could add the null check. and null terminate the result?
y = gsm_septet_encode(rdata, data); - *octets = gsm_septets2octets(result, rdata, y, 0); + + if (y > max_septets) + y = max_septets;
when would this happen? does this break an assumption of our code?
+int gsm_7bit_encode_n_ussd(uint8_t *result, size_t n, const char *data, int *octets) +{ + int y; + + y = gsm_7bit_encode_n(result, n, data, octets); + /* if last octet contains only one bit, add <CR> */ + if (((y * 7) & 7) == 1) + result[(*octets) - 1] |= ('\r' << 1);
okay, y >= 1 => *octets >= 1. But shouldn't we increase y in this case too?
+ .input = (const uint8_t *)concatenated_text,
i think we tend to put a ' ' after the ')'. But thanks for fixing all the warnings GCC started to emit. holger [1] http://www.stack.nl/~dimitri/doxygen/manual/grouping.html
The 7bit<->8bit encoding/decoding functions didn't check whether there is still enough space in the destination buffer. Therefore a buffer size parameter has been added to each of the functions which is used to truncate the output if the buffer is too small. In addition, the return value of the decoding functions has been changed to number of characters written (excluding \0), so this value is always equal to strlen(decoded). The old functions are still available as wrapper functions. --- include/osmocom/gsm/gsm_utils.h | 59 ++++++++++++++-- src/gsm/gsm0480.c | 11 ++- src/gsm/gsm_utils.c | 145 +++++++++++++++++++++++++-------------- src/gsm/libosmogsm.map | 7 +- tests/sms/sms_test.c | 124 ++++++++++++++++++--------------- tests/sms/sms_test.ok | 10 +++ tests/ussd/ussd_test.c | 50 ++++++++++++-- tests/ussd/ussd_test.ok | 11 +++ 8 files changed, 290 insertions(+), 127 deletions(-) diff --git a/include/osmocom/gsm/gsm_utils.h b/include/osmocom/gsm/gsm_utils.h index 6cd46e4..c111f94 100644 --- a/include/osmocom/gsm/gsm_utils.h +++ b/include/osmocom/gsm/gsm_utils.h @@ -25,6 +25,7 @@ #ifndef GSM_UTILS_H #define GSM_UTILS_H +#include <stddef.h> #include <stdint.h> #define ADD_MODULO(sum, delta, modulo) do { \ @@ -56,17 +57,54 @@ enum gsm_band { const char *gsm_band_name(enum gsm_band band); enum gsm_band gsm_band_parse(const char *mhz); -int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length); -int gsm_7bit_decode_ussd(char *decoded, const uint8_t *user_data, uint8_t length); -int gsm_7bit_decode_hdr(char *decoded, const uint8_t *user_data, uint8_t length, uint8_t ud_hdr_ind); -int gsm_7bit_encode(uint8_t *result, const char *data); -int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets_written); -int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets_written); +/*! + * \brief Decode a sequence of GSM 03.38 encoded 7 bit characters. + * + * \param decoded The destination buffer for the decoded characters. + * \param n A maximum of n chars is written (incl. terminating \0). + * Requires n >= 1. + * \param user_data A pointer to the start of the packed 7bit character + * sequence. + * \param length The length of the input sequence (in octets). + * + * \returns the number of (8 bit) chars written excluding the terminating \0. + * This is the same like strlen(decoded). + */ +int gsm_7bit_decode_n(char *decoded, size_t n, const uint8_t *user_data, uint8_t length); + +/*! + * \brief Decode a sequence of 7 bit characters (USSD encoding). + * + * \see gsm_7bit_encode_n() + */ +int gsm_7bit_decode_n_ussd(char *decoded, size_t n, const uint8_t *user_data, uint8_t length); + +/** + * \brief Encode a text string into GSM 03.38 encoded 7 bit characters. + * + * \param result The destination buffer for the packed 7 bit sequence. + * \param n A maximum of n octets is written. + * \param data A pointer to the start of the \0 terminated 8 bit character + * string. + * \param octets_written Iff not NULL, *octets_written will be set to the + * number of octets written to the result buffer. + * + * \returns the number of septets that have been created. + */ +int gsm_7bit_encode_n(uint8_t *result, size_t n, const char *data, int *octets_written); -/* the three functions below are helper functions and here for the unit test */ +/*! + * \brief Encode a text string into GSM 03.38 encoded 7 bit characters (USSD encoding). + * + * \see gsm_7bit_decode_n() + */ +int gsm_7bit_encode_n_ussd(uint8_t *result, size_t n, const char *data, int *octets_written); + +/* the four functions below are helper functions and here for the unit test */ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len, uint8_t padding); int gsm_septet_encode(uint8_t *result, const char *data); uint8_t gsm_get_octet_len(const uint8_t sept_len); +int gsm_7bit_decode_n_hdr(char *decoded, size_t n, const uint8_t *user_data, uint8_t length, uint8_t ud_hdr_ind); unsigned int ms_class_gmsk_dbm(enum gsm_band band, int ms_class); @@ -154,5 +192,12 @@ enum gsm_chan_t { _GSM_LCHAN_MAX }; +/* Deprectated functions */ +int gsm_7bit_decode(char *decoded, const uint8_t *user_data, uint8_t length) __attribute__((deprecated ("Use gsm_7bit_decode_n() instead"))); +int gsm_7bit_decode_ussd(char *decoded, const uint8_t *user_data, uint8_t length) __attribute__((deprecated ("Use gsm_7bit_decode_n_ussd() instead"))); +int gsm_7bit_encode(uint8_t *result, const char *data) __attribute__((deprecated ("Use gsm_7bit_encode_n() instead"))); +int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets_written) __attribute__((deprecated ("Use gsm_7bit_encode_n_ussd() instead"))); +int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets_written) __attribute__((deprecated ("Use gsm_7bit_encode_n() instead"))); + #endif diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c index cc693fe..92a62dc 100644 --- a/src/gsm/gsm0480.c +++ b/src/gsm/gsm0480.c @@ -105,7 +105,7 @@ struct msgb *gsm0480_create_unstructuredSS_Notify(int alertPattern, const char * msgb_put_u8(msg, ASN1_OCTET_STRING_TAG); ussd_len_ptr = msgb_put(msg, 1); data = msgb_put(msg, 0); - gsm_7bit_encode_ussd(data, text, &len); + gsm_7bit_encode_n_ussd(data, msgb_tailroom(msg), text, &len); msgb_put(msg, len); ussd_len_ptr[0] = len; /* USSD-String } */ @@ -172,7 +172,7 @@ struct msgb *gsm0480_create_notifySS(const char *text) msgb_put_u8(msg, 0x82); tmp_len = msgb_put(msg, 1); data = msgb_put(msg, 0); - gsm_7bit_encode_ussd(data, text, &len); + gsm_7bit_encode_n_ussd(data, msgb_tailroom(msg), text, &len); tmp_len[0] = len; msgb_put(msg, len); @@ -398,10 +398,7 @@ static int parse_process_uss_req(const uint8_t *uss_req_data, uint16_t length, if ((dcs == 0x0F) && (uss_req_data[5] == ASN1_OCTET_STRING_TAG)) { num_chars = (uss_req_data[6] * 8) / 7; - /* Prevent a mobile-originated buffer-overrun! */ - if (num_chars > MAX_LEN_USSD_STRING) - num_chars = MAX_LEN_USSD_STRING; - gsm_7bit_decode_ussd(req->text, + gsm_7bit_decode_n_ussd(req->text, sizeof(req->text), &(uss_req_data[7]), num_chars); rc = 1; } @@ -423,7 +420,7 @@ struct msgb *gsm0480_create_ussd_resp(uint8_t invoke_id, uint8_t trans_id, const /* First put the payload text into the message */ ptr8 = msgb_put(msg, 0); - gsm_7bit_encode_ussd(ptr8, text, &response_len); + gsm_7bit_encode_n_ussd(ptr8, msgb_tailroom(msg), text, &response_len); msgb_put(msg, response_len); /* Then wrap it as an Octet String */ diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index 3dd1537..e248078 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -28,7 +28,7 @@ * This library is a collection of common code used in various * GSM related sub-projects inside the Osmocom family of projects. It * includes A5/1 and A5/2 ciphers, COMP128v1, a LAPDm implementation, - * a GSM TLV parser, SMS utility routines as well as + * a GSM TLV parser, SMS utility routines as well as * protocol definitions for a series of protocols: * * Um L2 (04.06) * * Um L3 (04.08) @@ -123,12 +123,17 @@ uint8_t gsm_get_octet_len(const uint8_t sept_len){ } /* GSM 03.38 6.2.1 Character unpacking */ -int gsm_7bit_decode_hdr(char *text, const uint8_t *user_data, uint8_t septet_l, uint8_t ud_hdr_ind) +int gsm_7bit_decode_n_hdr(char *text, size_t n, const uint8_t *user_data, uint8_t septet_l, uint8_t ud_hdr_ind) { int i = 0; int shift = 0; - uint8_t c; + uint8_t c7, c8; uint8_t next_is_ext = 0; + const char *text_buf_begin = text; + const char *text_buf_end = text + n; + int nchars; + + OSMO_ASSERT (n > 0); /* skip the user data header */ if (ud_hdr_ind) { @@ -139,50 +144,49 @@ int gsm_7bit_decode_hdr(char *text, const uint8_t *user_data, uint8_t septet_l, septet_l = septet_l - shift; } - for (i = 0; i < septet_l; i++) { - c = + for (i = 0; i < septet_l && text != text_buf_end - 1; i++) { + c7 = ((user_data[((i + shift) * 7 + 7) >> 3] << (7 - (((i + shift) * 7 + 7) & 7))) | (user_data[((i + shift) * 7) >> 3] >> (((i + shift) * 7) & 7))) & 0x7f; - /* this is an extension character */ if (next_is_ext) { + /* this is an extension character */ next_is_ext = 0; - *(text++) = gsm_7bit_alphabet[0x7f + c]; - continue; - } - - if (c == 0x1b && i + 1 < septet_l) { + c8 = gsm_7bit_alphabet[0x7f + c7]; + } else if (c7 == 0x1b && i + 1 < septet_l) { next_is_ext = 1; + continue; } else { - *(text++) = gsm_septet_lookup(c); + c8 = gsm_septet_lookup(c7); } + + *(text++) = c8; } - if (ud_hdr_ind) - i += shift; + nchars = text - text_buf_begin; + *text = '\0'; - return i; + return nchars; } -int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t septet_l) +int gsm_7bit_decode_n(char *text, size_t n, const uint8_t *user_data, uint8_t septet_l) { - return gsm_7bit_decode_hdr(text, user_data, septet_l, 0); + return gsm_7bit_decode_n_hdr(text, n, user_data, septet_l, 0); } -int gsm_7bit_decode_ussd(char *text, const uint8_t *user_data, uint8_t length) +int gsm_7bit_decode_n_ussd(char *text, size_t n, const uint8_t *user_data, uint8_t length) { - int i; + int nchars; - gsm_7bit_decode_hdr(text, user_data, length, 0); - i = strlen(text); + nchars = gsm_7bit_decode_n_hdr(text, n, user_data, length, 0); /* remove last <CR>, if it fits up to the end of last octet */ - if (i && (user_data[gsm_get_octet_len(length) - 1] >> 1) == '\r') - text[--i] = '\0'; + if (nchars && (user_data[gsm_get_octet_len(length) - 1] >> 1) == '\r') + text[--nchars] = '\0'; - return i; + return nchars; } /* GSM 03.38 6.2.1 Prepare character packing */ @@ -261,38 +265,28 @@ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len } /* GSM 03.38 6.2.1 Character packing */ -int gsm_7bit_encode(uint8_t *result, const char *data) -{ - int out; - return gsm_7bit_encode_oct(result, data, &out); -} - -int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets) -{ - int y; - - y = gsm_7bit_encode_oct(result, data, octets); - /* if last octet contains only one bit, add <CR> */ - if (((y * 7) & 7) == 1) - result[(*octets) - 1] |= ('\r' << 1); - /* if last character is <CR> and completely fills last octet, add - * another <CR>. */ - if (y && ((y * 7) & 7) == 0 && (result[(*octets) - 1] >> 1) == '\r') { - result[(*octets)++] = '\r'; - y++; - } - - return y; -} - -int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) +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; /* prepare for the worst case, every character expanding to two bytes */ uint8_t *rdata = calloc(strlen(data) * 2, sizeof(uint8_t)); y = gsm_septet_encode(rdata, data); - *octets = gsm_septets2octets(result, rdata, y, 0); + + if (y > max_septets) { + /* + * Limit the number of septets to avoid the generation + * of more than n octets. + */ + y = max_septets; + } + + o = gsm_septets2octets(result, rdata, y, 0); + + if (octets) + *octets = o; free(rdata); @@ -309,6 +303,24 @@ int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) return y; } +int gsm_7bit_encode_n_ussd(uint8_t *result, size_t n, const char *data, int *octets) +{ + int y; + + y = gsm_7bit_encode_n(result, n, data, octets); + /* if last octet contains only one bit, add <CR> */ + if (((y * 7) & 7) == 1) + result[(*octets) - 1] |= ('\r' << 1); + /* if last character is <CR> and completely fills last octet, add + * another <CR>. */ + if (y && ((y * 7) & 7) == 0 && (result[(*octets) - 1] >> 1) == '\r' && *octets < n - 1) { + result[(*octets)++] = '\r'; + y++; + } + + return y; +} + /* convert power class to dBm according to GSM TS 05.05 */ unsigned int ms_class_gmsk_dbm(enum gsm_band band, int class) { @@ -373,7 +385,7 @@ int ms_pwr_ctl_lvl(enum gsm_band band, unsigned int dbm) case GSM_BAND_1800: if (dbm >= 36) return 29; - else if (dbm >= 34) + else if (dbm >= 34) return 30; else if (dbm >= 32) return 31; @@ -662,3 +674,34 @@ uint32_t gprs_tmsi2tlli(uint32_t p_tmsi, enum gprs_tlli_type type) } return tlli; } + +/* Wrappers for deprecated functions: */ + +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); + + /* Mimic the original behaviour. */ + return 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); +} + +int gsm_7bit_encode(uint8_t *result, const char *data) +{ + int out; + return gsm_7bit_encode_n(result, SIZE_MAX, 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); +} + +int gsm_7bit_encode_oct(uint8_t *result, const char *data, int *octets) +{ + return gsm_7bit_encode_n(result, SIZE_MAX, data, octets); +} diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index 1b985e1..9d15d66 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -133,11 +133,16 @@ gsm48_rr_att_tlvdef; gsm_7bit_decode; gsm_7bit_decode_ussd; -gsm_7bit_decode_hdr; gsm_7bit_encode; gsm_7bit_encode_ussd; gsm_7bit_encode_oct; +gsm_7bit_decode_n; +gsm_7bit_decode_n_ussd; +gsm_7bit_decode_n_hdr; +gsm_7bit_encode_n; +gsm_7bit_encode_n_ussd; + gsm_arfcn2band; gsm_arfcn2freq10; gsm_freq102arfcn; diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c index e48f9a3..3188a18 100644 --- a/tests/sms/sms_test.c +++ b/tests/sms/sms_test.c @@ -116,14 +116,14 @@ static const uint8_t concatenated_part2_enc[] = { static const struct test_case test_multiple_encode[] = { { - .input = concatenated_text, + .input = (const uint8_t *) concatenated_text, .expected = concatenated_part1_enc, .expected_octet_length = sizeof(concatenated_part1_enc), .expected_septet_length = concatenated_part1_septet_length, .ud_hdr_ind = 1, }, { - .input = concatenated_text, + .input = (const uint8_t *) concatenated_text, .expected = concatenated_part2_enc, .expected_octet_length = sizeof(concatenated_part2_enc), .expected_septet_length = concatenated_part2_septet_length, @@ -134,28 +134,28 @@ static const struct test_case test_multiple_encode[] = static const struct test_case test_encode[] = { { - .input = simple_text, + .input = (const uint8_t *) simple_text, .expected = simple_enc, .expected_octet_length = sizeof(simple_enc), .expected_septet_length = simple_septet_length, .ud_hdr_ind = 0, }, { - .input = escape_text, + .input = (const uint8_t *) escape_text, .expected = escape_enc, .expected_octet_length = sizeof(escape_enc), .expected_septet_length = escape_septet_length, .ud_hdr_ind = 0, }, { - .input = enhanced_text, + .input = (const uint8_t *) enhanced_text, .expected = enhanced_enc, .expected_octet_length = sizeof(enhanced_enc), .expected_septet_length = enhanced_septet_length, .ud_hdr_ind = 0, }, { - .input = enhancedV2_text, + .input = (const uint8_t *) enhancedV2_text, .expected = enhancedV2_enc, .expected_octet_length = sizeof(enhancedV2_enc), .expected_septet_length = enhancedV2_septet_length, @@ -168,42 +168,42 @@ static const struct test_case test_decode[] = { .input = simple_enc, .input_length = sizeof(simple_enc), - .expected = simple_text, + .expected = (const uint8_t *) simple_text, .expected_septet_length = simple_septet_length, .ud_hdr_ind = 0, }, { .input = escape_enc, .input_length = sizeof(escape_enc), - .expected = escape_text, + .expected = (const uint8_t *) escape_text, .expected_septet_length = escape_septet_length, .ud_hdr_ind = 0, }, { .input = enhanced_enc, .input_length = sizeof(enhanced_enc), - .expected = enhanced_text, + .expected = (const uint8_t *) enhanced_text, .expected_septet_length = enhanced_septet_length, .ud_hdr_ind = 0, }, { .input = enhancedV2_enc, .input_length = sizeof(enhancedV2_enc), - .expected = enhancedV2_text, + .expected = (const uint8_t *) enhancedV2_text, .expected_septet_length = enhancedV2_septet_length, .ud_hdr_ind = 0, }, { .input = concatenated_part1_enc, .input_length = sizeof(concatenated_part1_enc), - .expected = splitted_text_part1, + .expected = (const uint8_t *) splitted_text_part1, .expected_septet_length = concatenated_part1_septet_length_with_header, .ud_hdr_ind = 1, }, { .input = concatenated_part2_enc, .input_length = sizeof(concatenated_part2_enc), - .expected = splitted_text_part2, + .expected = (const uint8_t *) splitted_text_part2, .expected_septet_length = concatenated_part2_septet_length_with_header, .ud_hdr_ind = 1, }, @@ -216,7 +216,7 @@ static void test_octet_return() printf("Encoding some tests and printing number of septets/octets\n"); - septets = gsm_7bit_encode_oct((uint8_t *) out, "test1234", &oct); + septets = gsm_7bit_encode_n((uint8_t *) out, sizeof(out), "test1234", &oct); printf("SEPTETS: %d OCTETS: %d\n", septets, oct); printf("Done\n"); @@ -227,43 +227,58 @@ int main(int argc, char** argv) printf("SMS testing\n"); struct msgb *msg; uint8_t i; - + uint16_t buffer_size; uint8_t octet_length; + int octets_written; + uint8_t computed_octet_length; uint8_t septet_length; uint8_t gsm_septet_length; uint8_t coded[256]; uint8_t tmp[160]; uint8_t septet_data[256]; uint8_t ud_header[6]; + int nchars; char result[256]; /* test 7-bit encoding */ for (i = 0; i < ARRAY_SIZE(test_encode); ++i) { memset(coded, 0x42, sizeof(coded)); - septet_length = gsm_7bit_encode(coded, test_encode[i].input); - octet_length = gsm_get_octet_len(septet_length); - if (octet_length != test_encode[i].expected_octet_length) { - fprintf(stderr, "Encode case %d: Octet length failure. Got %d, expected %d\n", - i, octet_length, test_encode[i].expected_octet_length); - return -1; - } - - if (septet_length != test_encode[i].expected_septet_length){ - fprintf(stderr, "Encode case %d: Septet length failure. Got %d, expected %d\n", - i, septet_length, test_encode[i].expected_septet_length); - return -1; - } - - if (memcmp(coded, test_encode[i].expected, octet_length) != 0) { - fprintf(stderr, "Encoded content does not match for case %d\n", - i); - return -1; + septet_length = gsm_7bit_encode_n(coded, sizeof(coded), + (const char *) test_encode[i].input, + &octets_written); + computed_octet_length = gsm_get_octet_len(septet_length); + printf("Encode case %d: " + "Octet length %d (expected %d, computed %d), " + "septet length %d (expected %d)\n" + , i + , octets_written, test_encode[i].expected_octet_length, computed_octet_length + , septet_length, test_encode[i].expected_septet_length + ); + + 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); + + /* check buffer limiting */ + memset(coded, 0xaa, sizeof(coded)); + + for (buffer_size = 0; + buffer_size < test_encode[i].expected_octet_length + 1 + && buffer_size < sizeof(coded) - 1; + ++buffer_size) + { + gsm_7bit_encode_n(coded, buffer_size, + (const char *) test_encode[i].input, + &octets_written); + + OSMO_ASSERT(octets_written <= buffer_size); + OSMO_ASSERT(coded[buffer_size] == 0xaa); } } /* Test: encode multiple SMS */ - int number_of_septets = gsm_septet_encode(septet_data, test_multiple_encode[0].input); + int number_of_septets = gsm_septet_encode(septet_data, (const char *) test_multiple_encode[0].input); /* SMS part 1 */ memset(tmp, 0x42, sizeof(tmp)); @@ -281,11 +296,7 @@ int main(int argc, char** argv) memset(coded, 0x42, sizeof(coded)); memcpy(coded, tmp, octet_length + 6); - if (memcmp(coded, test_multiple_encode[0].expected, octet_length) != 0) { - fprintf(stderr, "Multiple-SMS encoded content does not match for part 1\n"); - return -1; - } - + OSMO_ASSERT(memcmp(coded, test_multiple_encode[0].expected, octet_length) == 0); /* SMS part 2 */ memset(tmp, 0x42, sizeof(tmp)); @@ -303,27 +314,32 @@ int main(int argc, char** argv) memset(coded, 0x42, sizeof(coded)); memcpy(coded, tmp, octet_length + 6); - if (memcmp(coded, test_multiple_encode[1].expected, octet_length) != 0) { - fprintf(stderr, "Multiple-SMS encoded content does not match for part 2\n"); - return -1; - } - - + OSMO_ASSERT(memcmp(coded, test_multiple_encode[1].expected, octet_length) == 0); /* test 7-bit decoding */ for (i = 0; i < ARRAY_SIZE(test_decode); ++i) { - memset(result, 0x42, sizeof(coded)); - septet_length = gsm_7bit_decode_hdr(result, test_decode[i].input, + 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); + printf("Decode case %d: return value %d (expected %d)\n", i, nchars, strlen(result)); - if (strcmp(result, test_decode[i].expected) != 0) { - fprintf(stderr, "Test case %d failed to decode.\n", i); - return -1; - } - if (septet_length != test_decode[i].expected_septet_length) { - fprintf(stderr, "Decode case %d: Septet length failure. Got %d, expected %d\n", - i, septet_length, test_decode[i].expected_septet_length); - return -1; + OSMO_ASSERT(strcmp(result, (const char *) test_decode[i].expected) == 0); + OSMO_ASSERT(nchars == strlen(result)); + + /* check buffer limiting */ + memset(result, 0xaa, sizeof(result)); + + for (buffer_size = 1; + buffer_size < test_encode[i].expected_septet_length + 1 + && buffer_size < sizeof(result) - 1; + ++buffer_size) + { + nchars = gsm_7bit_decode_n_hdr(result, buffer_size, test_decode[i].input, + test_decode[i].expected_septet_length, test_decode[i].ud_hdr_ind); + + OSMO_ASSERT(nchars <= buffer_size); + OSMO_ASSERT(result[buffer_size] == (char)0xaa); + OSMO_ASSERT(result[nchars] == '\0'); } } diff --git a/tests/sms/sms_test.ok b/tests/sms/sms_test.ok index ce6cb17..915a59c 100644 --- a/tests/sms/sms_test.ok +++ b/tests/sms/sms_test.ok @@ -1,4 +1,14 @@ SMS testing +Encode case 0: Octet length 8 (expected 8, computed 8), septet length 9 (expected 9) +Encode case 1: Octet length 36 (expected 36, computed 36), septet length 41 (expected 41) +Encode case 2: Octet length 35 (expected 35, computed 35), septet length 39 (expected 39) +Encode case 3: Octet length 35 (expected 35, computed 35), septet length 40 (expected 40) +Decode case 0: return value 9 (expected 9) +Decode case 1: return value 40 (expected 40) +Decode case 2: return value 31 (expected 31) +Decode case 3: return value 32 (expected 32) +Decode case 4: return value 153 (expected 153) +Decode case 5: return value 40 (expected 40) Encoding some tests and printing number of septets/octets SEPTETS: 8 OCTETS: 7 Done diff --git a/tests/ussd/ussd_test.c b/tests/ussd/ussd_test.c index e3e8e08..b04f8e8 100644 --- a/tests/ussd/ussd_test.c +++ b/tests/ussd/ussd_test.c @@ -73,20 +73,42 @@ static void test_7bit_ussd(const char *text, const char *encoded_hex, const char { uint8_t coded[256]; char decoded[256]; - int y; + int octets_written; + int buffer_size; + int nchars; printf("original = %s\n", osmo_hexdump((uint8_t *)text, strlen(text))); - gsm_7bit_encode_ussd(coded, text, &y); - printf("encoded = %s\n", osmo_hexdump(coded, y)); + gsm_7bit_encode_n_ussd(coded, sizeof(coded), text, &octets_written); + printf("encoded = %s\n", osmo_hexdump(coded, octets_written)); - OSMO_ASSERT(strcmp(encoded_hex, osmo_hexdump_nospc(coded, y)) == 0); + OSMO_ASSERT(strcmp(encoded_hex, osmo_hexdump_nospc(coded, octets_written)) == 0); - gsm_7bit_decode_ussd(decoded, coded, y * 8 / 7); - y = strlen(decoded); - printf("decoded = %s\n\n", osmo_hexdump((uint8_t *)decoded, y)); + gsm_7bit_decode_n_ussd(decoded, sizeof(decoded), coded, octets_written * 8 / 7); + octets_written = strlen(decoded); + printf("decoded = %s\n\n", osmo_hexdump((uint8_t *)decoded, octets_written)); OSMO_ASSERT(strncmp(text, decoded, strlen(text)) == 0); OSMO_ASSERT(strcmp(appended_after_decode, decoded + strlen(text)) == 0); + + /* check buffer limiting */ + memset(decoded, 0xaa, sizeof(decoded)); + + for (buffer_size = 1; buffer_size < sizeof(decoded) - 1; ++buffer_size) + { + nchars = gsm_7bit_decode_n_ussd(decoded, buffer_size, coded, octets_written * 8 / 7); + OSMO_ASSERT(nchars <= buffer_size); + OSMO_ASSERT(decoded[buffer_size] == (char)0xaa); + OSMO_ASSERT(decoded[nchars] == '\0'); + } + + memset(coded, 0xaa, sizeof(coded)); + + for (buffer_size = 0; buffer_size < sizeof(coded) - 1; ++buffer_size) + { + gsm_7bit_encode_n_ussd(coded, buffer_size, text, &octets_written); + OSMO_ASSERT(octets_written <= buffer_size); + OSMO_ASSERT(coded[buffer_size] == 0xaa); + } } int main(int argc, char **argv) @@ -94,6 +116,7 @@ int main(int argc, char **argv) struct ussd_request req; const int size = sizeof(ussd_request); int i; + struct msgb *msg; osmo_init_logging(&info); @@ -122,5 +145,18 @@ int main(int argc, char **argv) test_7bit_ussd("0123456\r", "b0986c46abd91a0d", "\r"); test_7bit_ussd("012345\r", "b0986c46ab351a", ""); + printf("Checking GSM 04.80 USSD message generation.\n"); + + test_7bit_ussd("", "", ""); + msg = gsm0480_create_unstructuredSS_Notify (0x00, ""); + printf ("Created unstructuredSS_Notify (0x00): %s\n", + osmo_hexdump(msgb_data(msg), msgb_length(msg))); + msgb_free (msg); + + test_7bit_ussd("forty-two", "e6b79c9e6fd1ef6f", ""); + msg = gsm0480_create_unstructuredSS_Notify (0x42, "forty-two"); + printf ("Created unstructuredSS_Notify (0x42): %s\n", + osmo_hexdump(msgb_data(msg), msgb_length(msg))); + msgb_free (msg); return 0; } diff --git a/tests/ussd/ussd_test.ok b/tests/ussd/ussd_test.ok index 91f2a31..54d59ee 100644 --- a/tests/ussd/ussd_test.ok +++ b/tests/ussd/ussd_test.ok @@ -72,3 +72,14 @@ original = 30 31 32 33 34 35 0d encoded = b0 98 6c 46 ab 35 1a decoded = 30 31 32 33 34 35 0d +Checking GSM 04.80 USSD message generation. +original = +encoded = +decoded = + +Created unstructuredSS_Notify (0x00): 30 08 04 01 0f 04 00 04 01 00 +original = 66 6f 72 74 79 2d 74 77 6f +encoded = e6 b7 9c 9e 6f d1 ef 6f +decoded = 66 6f 72 74 79 2d 74 77 6f + +Created unstructuredSS_Notify (0x42): 30 10 04 01 0f 04 08 e6 b7 9c 9e 6f d1 ef 6f 04 01 42 -- 1.7.9.5
The table structure is not intuitive, so this comment shall aid to understand this. --- src/gsm/gsm_utils.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index e248078..5241c91 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -80,8 +80,12 @@ * left out as they can't be handled with a char and * since most phones don't display or write these * characters this would only needlessly make the code - * more complex -*/ + * more complex. + * + * Note that this table contains the latin1->7bit mapping _and_ has + * been merged with the reverse mapping (7bit->latin1) for the + * extended characters at offset 0x7f. + */ static unsigned char gsm_7bit_alphabet[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x0a, 0xff, 0xff, 0x0d, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, -- 1.7.9.5
participants (2)
-
Holger Hans Peter Freyther -
Jacob Erlbeck