Hello

Our goal was to send status sms via the vty interface. But all of our sms were cropped. In contrast sms from one MS to another MS are displayed correctly (despite the fact, that the text at the database contains several '@' at the end / the user_data contains several zero-octets). Therefore i have inspect the code and found several bugs.

The main problem is that the "user_data_len" is not correctly used. As per GSM 03.40, 9.2.3.16 TP‑User‑Data‑Length (TP‑UDL): "If the TP‑User‑Data is coded using the GSM 7 bit default alphabet, the TP‑User‑Data‑Length field gives an integer representation of the number of septets within the TP‑User‑Data field to follow."

Currently the "user_data_len" contains the number of octets (returned from gsm_7bit_encode(...) at gsm_utils.c (libosmocore)).
The big problem here is that this information is not unique, e.g.:
1.) 46 non-extension characters + 1 extension character => (46 * 7 bit + (1 * (2 * 7 bit))) / 8 bit = 42 octets
2.) 47 non-extension characters => (47 * 7 bit) / 8 bit = 41,125 = 42 octets
3.) 48 non-extension characters => (48 * 7 bit) / 8 bit = 42 octects

But the MS has to know the correct "user_data_len" to decode the correct number of characters. For this reason i updated the gsm_7bit_encode() function to return the correct number of septets. However sometimes it is needed to know the correct number of octets (e.g. at gsm_04_11.c: gsm340_gen_tpdu(...)) => i added a function to gsm_utils.c named:
uint8_t get_octet_len(const uint8_t sept_len)


I have also fixed the problem, that the sms are wrongly stored / displayed on the database. But the solution on the function *sms_from_result(...) (at db.c) is not really "beautiful". This is because there exists no "user_data_len" field at the database. To store the right value for "user_data_len" (which is further needed) i have to get the length from the "text" field. Unfortunately this is not enough. If the text contains extension characters like {[]} etc. then the "user_data_len" has to be bigger because these characters needs two septets. Therefore i use a switch statement so search for these characters. A better solution for that is to store the right "user_data_len" to the database (on the encoding / decoding procedure). But i don't know if this is a suitable solution for all of you (because you have to change your database structure etc.).


Best Regards
Dennis Wehrle


diff --git a/openbsc/src/libmsc/db.c b/openbsc/src/libmsc/db.c
index 95a7d36..caeded5 100644
--- a/openbsc/src/libmsc/db.c
+++ b/openbsc/src/libmsc/db.c
@@ -996,8 +996,10 @@ int db_sms_store(struct gsm_sms *sms)
 
        dbi_conn_quote_string_copy(conn, (char *)sms->text, &q_text);
        dbi_conn_quote_string_copy(conn, (char *)sms->dest_addr, &q_daddr);
-       dbi_conn_quote_binary_copy(conn, sms->user_data, sms->user_data_len,
-                                  &q_udata);
+
+       int octet_len = get_octet_len(sms->user_data_len);
+       dbi_conn_quote_binary_copy(conn, sms->user_data, octet_len, &q_udata);
+
        /* FIXME: correct validity period */
        result = dbi_conn_queryf(conn,
                "INSERT INTO SMS "
@@ -1057,17 +1059,40 @@ static struct gsm_sms *sms_from_result(struct 
gsm_network *net, dbi_result resul
                sms->dest_addr[sizeof(sms->dest_addr)-1] = '\0';
        }
 
-       sms->user_data_len = dbi_result_get_field_length(result, "user_data");
-       user_data = dbi_result_get_binary(result, "user_data");
-       if (sms->user_data_len > sizeof(sms->user_data))
-               sms->user_data_len = (u_int8_t) sizeof(sms->user_data);
-       memcpy(sms->user_data, user_data, sms->user_data_len);
-
        text = dbi_result_get_string(result, "text");
        if (text) {
                strncpy(sms->text, text, sizeof(sms->text));
                sms->text[sizeof(sms->text)-1] = '\0';
        }
+
+       // user_data_len contains the number of character, therefore use the 
"text"-field to get user_data_len.
+       // But each extension character has to be count twice, because they 
need 2 septets.
+       sms->user_data_len = strlen(text);
+       int i;
+       for(i = 0; i < strlen(text); i++){
+               switch(text[i]){
+               case '|':               /* character: | */
+               case '^':               /* character: ^ */
+               case '€':             /* character: € */
+               case '{':               /* character: { */
+               case '}':               /* character: } */
+               case '[':               /* character: [ */
+               case ']':               /* character: ] */
+               case '~':               /* character: ~ */
+               case '\\':              /* character: \ */
+                       sms->user_data_len++;
+               default:
+                       continue;
+               }
+       }
+       // if we don't want to use the switch statement, we could use 
gsm_7bit_encode which tells us the correct length:
+       // sms->user_data_len = gsm_7bit_encode(sms->user_data, text);
+
+       user_data = dbi_result_get_binary(result, "user_data");
+       if (sms->user_data_len > sizeof(sms->user_data))
+               sms->user_data_len = (u_int8_t) sizeof(sms->user_data);
+       memcpy(sms->user_data, user_data, sms->user_data_len);
+
        return sms;
 }
 
diff --git a/openbsc/src/libmsc/gsm_04_11.c b/openbsc/src/libmsc/gsm_04_11.c
index 812e758..9111d22 100644
--- a/openbsc/src/libmsc/gsm_04_11.c
+++ b/openbsc/src/libmsc/gsm_04_11.c
@@ -125,7 +125,10 @@ struct gsm_sms *sms_from_text(struct gsm_subscriber 
*receiver, int dcs, const ch
                return NULL;
 
        sms->receiver = subscr_get(receiver);
-       strncpy(sms->text, text, sizeof(sms->text)-1);
+
+       // ATM: because you can't send multiple sms from the terminal, copy 
only max 160 characters
+       strncpy(sms->text, text, 160);
+       //strncpy(sms->text, text, sizeof(sms->text)-1);
 
        /* FIXME: don't use ID 1 static */
        sms->sender = subscr_get_by_id(receiver->net, 1);
@@ -526,9 +529,7 @@ static int gsm340_gen_tpdu(struct msgb *msg, struct gsm_sms 
*sms)
        /* generate TP-UD */
        switch (gsm338_get_sms_alphabet(sms->data_coding_scheme)) {
        case DCS_7BIT_DEFAULT:
-               octet_len = sms->user_data_len*7/8;
-               if (sms->user_data_len*7%8 != 0)
-                       octet_len++;
+               octet_len = get_octet_len(sms->user_data_len);
                /* Warning, user_data_len indicates the amount of septets
                 * (characters), we need amount of octets occupied */
                smsp = msgb_put(msg, octet_len);
diff --git a/openbsc/src/libmsc/gsm_04_80.c b/openbsc/src/libmsc/gsm_04_80.c
index 494c319..27b4ce6 100644
--- a/openbsc/src/libmsc/gsm_04_80.c
+++ b/openbsc/src/libmsc/gsm_04_80.c
@@ -73,7 +73,9 @@ int gsm0480_send_ussd_response(struct 
gsm_subscriber_connection *conn,
        /* First put the payload text into the message */
        ptr8 = msgb_put(msg, 0);
        response_len = gsm_7bit_encode(ptr8, response_text);
-       msgb_put(msg, response_len);
+
+       int octet_len = get_octet_len(response_len);
+       msgb_put(msg, octet_len);
 
        /* Then wrap it as an Octet String */
        msgb_wrap_with_TL(msg, ASN1_OCTET_STRING_TAG);
diff --git a/include/osmocore/gsm_utils.h b/include/osmocore/gsm_utils.h
index 0aadd2e..2fe4af4 100644
--- a/include/osmocore/gsm_utils.h
+++ b/include/osmocore/gsm_utils.h
@@ -59,6 +59,8 @@ 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_encode(uint8_t *result, const char *data);
 
+uint8_t get_octet_len(const uint8_t sept_len);
+
 int ms_pwr_ctl_lvl(enum gsm_band band, unsigned int dbm);
 int ms_pwr_dbm(enum gsm_band band, uint8_t lvl);
 
diff --git a/src/gsm_utils.c b/src/gsm_utils.c
index 31e3cd6..40d6fb6 100644
--- a/src/gsm_utils.c
+++ b/src/gsm_utils.c
@@ -73,17 +73,29 @@ static int gsm_septet_lookup(uint8_t ch)
        return -1;
 }
 
+/* Warning, user_data_len indicates the amount of septets (characters), we 
need amount of octets occupied */
+/* compute the number of octets: */
+/* #octets = (#septets * 7 bit) / 8 bit */
+/* e.g. 47 non-extension characters needs 329 bits / 8 bit = 41,125 octets => 
we need 42 octets for 47 characters */
+uint8_t get_octet_len(const uint8_t sept_len){
+       int octet_len = (sept_len * 7) / 8;
+       if ((sept_len * 7) % 8 != 0)
+               octet_len++;
+
+       return octet_len;
+}
+
 /* GSM 03.38 6.2.1 Character unpacking */
 int gsm_7bit_decode(char *text, const uint8_t *user_data, uint8_t length)
 {
        int i = 0;
-       int l = 0;
-       int septet_l = (length * 8) / 7;
+
+       // length is equal to the number of septets, therefore we don't need to 
calculate it
+       int septet_l = length;
        uint8_t *rtext = calloc(septet_l, sizeof(uint8_t));
        uint8_t tmp;
 
        /* FIXME: We need to account for user data headers here */
-       i += l;
        for (; i < septet_l; i++){
                rtext[i] =
                        ((user_data[(i * 7 + 7) >> 3] <<
@@ -155,6 +167,11 @@ int gsm_7bit_encode(uint8_t *result, const char *data)
                        shift = 0;
                        continue;
                }
+               // special end case. This is necessary if the last septet fits 
into the previous octet. E.g. 48 non-extension characters: ....ag ( a = 
1100001, g = 1100111)
+               // result[40] = 100001 XX
+               // result[41] = 1100111 1
+               if(shift == 7 && i + 1 == y)
+                       break;
 
                cb = (rdata[i] & 0x7f) >> shift;
                if(i + 1 < y){
@@ -168,7 +185,13 @@ int gsm_7bit_encode(uint8_t *result, const char *data)
        }
 
        free(rdata);
-       return z;
+
+       // we don't care about the number of octets, because they are not 
unique. E.g.:
+       // 1.) 46 non-extension characters + 1 extension character => (46 * 7 
bit + (1 * (2 * 7 bit))) / 8 bit =  42 octets
+       // 2.) 47 non-extension characters => (47 * 7 bit) / 8 bit = 41,125 = 
42 octets
+       // 3.) 48 non-extension characters => (48 * 7 bit) / 8 bit = 42 octects
+       return y;
+       //return z;
 }
 
 /* determine power control level for given dBm value, as indicated

Reply via email to