pespin has uploaded this change for review. ( 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 adam.bedard@gmail.com Change-Id: I4baa19007c65275ead3d1fc92462bfb8ab68e036 --- M include/osmocom/gsm/gsm_utils.h M src/gsm/gsm_utils.c M src/gsm/libosmogsm.map M tests/sms/sms_test.c 4 files changed, 26 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/52/43252/1
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..27ecad2 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,8 @@ cb = cb | nb; }
+ if (z == result_size) + return -ENOBUFS; result[z++] = cb; shift++; } @@ -367,10 +371,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 +412,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));