pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/43252?usp=email )
Change subject: gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack() ...................................................................... gsm: Introduce gsm_septet_pack2() and deprecate gsm_septet_pack() The new gsm_septet_pack2() comes with a new parameter containing the size of the output buffer, effectively protecting against write buffer overflows. Related: OS#7059 Reported-By: Adam Bedard <[email protected]> Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 --- M TODO-RELEASE M include/osmocom/gsm/gsm_utils.h M src/gsm/gsm_utils.c M src/gsm/libosmogsm.map M tests/sms/sms_test.c 5 files changed, 29 insertions(+), 8 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/TODO-RELEASE b/TODO-RELEASE index 0ed7189..bd40b26 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,4 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +gsm add API gsm_septet_pack2() diff --git a/include/osmocom/gsm/gsm_utils.h b/include/osmocom/gsm/gsm_utils.h index 72bd131..e31e559 100644 --- a/include/osmocom/gsm/gsm_utils.h +++ b/include/osmocom/gsm/gsm_utils.h @@ -109,8 +109,10 @@ /* 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) OSMO_DEPRECATED("This function is unable to handle more than 255 septets, " - "use gsm_septet_pack() instead."); -int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding); + "use gsm_septet_pack2() instead."); +int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding) + OSMO_DEPRECATED("This function is not write-safe, use gsm_septet_pack2() instead."); +int gsm_septet_pack2(uint8_t *result, size_t result_size, const uint8_t *rdata, size_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); diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c index 3ca6812..117b2de 100644 --- a/src/gsm/gsm_utils.c +++ b/src/gsm/gsm_utils.c @@ -90,6 +90,7 @@ #include <errno.h> #include <ctype.h> #include <inttypes.h> +#include <limits.h> #include <time.h> #include <unistd.h> @@ -317,11 +318,12 @@ /*! GSM Default Alphabet 7bit to octet packing * \param[out] result Caller-provided output buffer + * \param[in] result_size Caller-provided output buffer size * \param[in] rdata Input data septets * \param[in] septet_len Length of \a rdata * \param[in] padding padding bits at start - * \returns number of bytes used in \a result */ -int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding) + * \returns number of bytes used in \a result, negative on error */ +int gsm_septet_pack2(uint8_t *result, size_t result_size, const uint8_t *rdata, size_t septet_len, uint8_t padding) { int i = 0, z = 0; uint8_t cb, nb; @@ -358,6 +360,10 @@ cb = cb | nb; } + if (z == result_size) { + free(data); + return -ENOBUFS; + } result[z++] = cb; shift++; } @@ -367,10 +373,21 @@ return z; } +/*! GSM Default Alphabet 7bit to octet packing + * \param[out] result Caller-provided output buffer + * \param[in] rdata Input data septets + * \param[in] septet_len Length of \a rdata + * \param[in] padding padding bits at start + * \returns number of bytes used in \a result, negative on error */ +int gsm_septet_pack(uint8_t *result, const uint8_t *rdata, size_t septet_len, uint8_t padding) +{ + return gsm_septet_pack2(result, INT_MAX, rdata, septet_len, padding); +} + /*! Backwards compatibility wrapper for gsm_septets_pack(), deprecated. */ int gsm_septets2octets(uint8_t *result, const uint8_t *rdata, uint8_t septet_len, uint8_t padding) { - return gsm_septet_pack(result, rdata, septet_len, padding); + return gsm_septet_pack2(result, INT_MAX, rdata, septet_len, padding); } /*! GSM 7-bit alphabet TS 03.38 6.2.1 Character packing @@ -397,7 +414,7 @@ y = max_septets; } - o = gsm_septet_pack(result, rdata, y, 0); + o = gsm_septet_pack2(result, n, rdata, y, 0); if (octets) *octets = o; diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index 78e0385..c57d286 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -566,6 +566,7 @@ gsm_milenage; gsm_septet_encode; gsm_septet_pack; +gsm_septet_pack2; gsm_septets2octets; lapd_dl_exit; diff --git a/tests/sms/sms_test.c b/tests/sms/sms_test.c index 912c082..dda3c56 100644 --- a/tests/sms/sms_test.c +++ b/tests/sms/sms_test.c @@ -380,7 +380,7 @@ memcpy(tmp, septet_data, concatenated_part1_septet_length); /* In our case: test_multiple_decode[0].ud_hdr_ind equals number of padding bits*/ - octet_length = gsm_septet_pack(coded, tmp, concatenated_part1_septet_length, test_multiple_encode[0].ud_hdr_ind); + octet_length = gsm_septet_pack2(coded, sizeof(coded), tmp, concatenated_part1_septet_length, test_multiple_encode[0].ud_hdr_ind); /* copy header */ memset(tmp, 0x42, sizeof(tmp)); @@ -398,7 +398,7 @@ memcpy(tmp, septet_data + concatenated_part1_septet_length, concatenated_part2_septet_length); /* In our case: test_multiple_decode[1].ud_hdr_ind equals number of padding bits*/ - octet_length = gsm_septet_pack(coded, tmp, concatenated_part2_septet_length, test_multiple_encode[1].ud_hdr_ind); + octet_length = gsm_septet_pack2(coded, sizeof(coded), tmp, concatenated_part2_septet_length, test_multiple_encode[1].ud_hdr_ind); /* copy header */ memset(tmp, 0x42, sizeof(tmp)); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43252?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 Gerrit-Change-Number: 43252 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-Reviewer: pespin <[email protected]>
