[MERGED] libsmpp34[master]: Fix Out of bounds compilation warning in OCTET8

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Sun Sep 17 14:59:45 UTC 2017


Pau Espin Pedrol has submitted this change and it was merged.

Change subject: Fix Out of bounds compilation warning in OCTET8
......................................................................


Fix Out of bounds compilation warning in OCTET8

The code in OCTET8 implementation assumes the len is placed inside the
byte preceding the memory buffer, which is true for the defined cases.
However, it creates a compilation warning. Better pass the value
directly from the struct field rather than playing addr games. this way
we also assert we require to explicitly pass the len.

Fixes lots of warning like the one below:
/home/pespin/dev/sysmocom/bin/../git/libsmpp34/src/smpp34_unpack.c: In function ‘smpp34_u
npack’:
/home/pespin/dev/sysmocom/bin/../git/libsmpp34/src/smpp34_unpack.c:147:14: warning: array
 subscript is above array bounds [-Warray-bounds]
     lenval = *((inst par) - 1);\
              ^~~~~~~~~~~~~~~~~
/home/pespin/dev/sysmocom/bin/../git/libsmpp34/def_frame/submit_sm.frame:18:2: note: in e
xpansion of macro ‘OCTET8’
  OCTET8( instancia, short_message, 254 );
  ^~~~~~

Change-Id: Id110f4e977c3becdb44cf5492c372e530ea51551
---
M def_frame/deliver_sm.frame
M def_frame/replace_sm.frame
M def_frame/submit_multi.frame
M def_frame/submit_sm.frame
M src/smpp34_dumpPdu.c
M src/smpp34_pack.c
M src/smpp34_structs.h
M src/smpp34_unpack.c
8 files changed, 8 insertions(+), 11 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/def_frame/deliver_sm.frame b/def_frame/deliver_sm.frame
index 2306f19..6cbd90d 100644
--- a/def_frame/deliver_sm.frame
+++ b/def_frame/deliver_sm.frame
@@ -15,5 +15,5 @@
     U08( instancia, data_coding, valueDec_08 );
     U08( instancia, sm_default_msg_id, valueDec_08 );
     U08( instancia, sm_length, valueDec_08 );
- OCTET8( instancia, short_message, 254 );
+ OCTET8( instancia, short_message, 254, instancia sm_length );
     TLV( instancia, tlv, do_tlv_deliver_sm );
diff --git a/def_frame/replace_sm.frame b/def_frame/replace_sm.frame
index a35187d..641207e 100644
--- a/def_frame/replace_sm.frame
+++ b/def_frame/replace_sm.frame
@@ -7,4 +7,4 @@
     U08( instancia, registered_delivery, valueDec_08 );
     U08( instancia, sm_default_msg_id, valueDec_08 );
     U08( instancia, sm_length, valueDec_08 );
- OCTET8( instancia, short_message, 254 );
+ OCTET8( instancia, short_message, 254, instancia sm_length );
diff --git a/def_frame/submit_multi.frame b/def_frame/submit_multi.frame
index 0c5fb31..e58fd36 100644
--- a/def_frame/submit_multi.frame
+++ b/def_frame/submit_multi.frame
@@ -14,5 +14,5 @@
     U08( instancia, data_coding, valueDec_08 );
     U08( instancia, sm_default_msg_id, valueDec_08 );
     U08( instancia, sm_length, valueDec_08 );
- OCTET8( instancia, short_message, 254 );
+ OCTET8( instancia, short_message, 254, instancia sm_length );
     TLV( instancia, tlv, do_tlv_submit_multi );
diff --git a/def_frame/submit_sm.frame b/def_frame/submit_sm.frame
index 0a54421..bf8e560 100644
--- a/def_frame/submit_sm.frame
+++ b/def_frame/submit_sm.frame
@@ -15,5 +15,5 @@
     U08( instancia, data_coding, valueDec_08 );
     U08( instancia, sm_default_msg_id, valueDec_08 );
     U08( instancia, sm_length, valueDec_08 );
- OCTET8( instancia, short_message, 254 );
+ OCTET8( instancia, short_message, 254, instancia sm_length );
     TLV( instancia, tlv, do_tlv_submit_sm );
diff --git a/src/smpp34_dumpPdu.c b/src/smpp34_dumpPdu.c
index 89d6e53..688f4ea 100644
--- a/src/smpp34_dumpPdu.c
+++ b/src/smpp34_dumpPdu.c
@@ -136,11 +136,10 @@
     _op(inst, par, size )\
 }
 
-#define OCTET8( inst, par, size ){\
+#define OCTET8( inst, par, size, lenval ){\
     int i = 0;\
     uint8_t *p = l_dest;\
     int dummy = 0;\
-    lenval = *((inst par) - 1);\
     if( (lenval + 33) >= left ){\
         PUTLOG("[%s:%s(%s)]", par, inst par, \
                                      "Value length exceed buffer length");\
diff --git a/src/smpp34_pack.c b/src/smpp34_pack.c
index b36e7a4..ef1c599 100644
--- a/src/smpp34_pack.c
+++ b/src/smpp34_pack.c
@@ -139,8 +139,7 @@
     }\
 };
 
-#define OCTET8( inst, par, sizeval ){\
-    lenval = *((inst par) - 1);\
+#define OCTET8( inst, par, sizeval, lenval ){\
     if( lenval >= left ){\
         PUTLOG("[leng %s:%d(%s)]", par, lenval,\
                                       "Value length exceed buffer length");\
diff --git a/src/smpp34_structs.h b/src/smpp34_structs.h
index 71d22cd..7c83df8 100644
--- a/src/smpp34_structs.h
+++ b/src/smpp34_structs.h
@@ -75,7 +75,7 @@
 
 #define O_C_OCTET( inst, par, size ) uint8_t par[ size ];
 #define C_OCTET( inst, par, size ) uint8_t par[ size ];
-#define OCTET8( inst, par, size ) uint8_t par[ size ];
+#define OCTET8( inst, par, size, lenval ) uint8_t par[ size ];
 #define OCTET16( inst, par, size ) uint8_t par[ size ];
 
 #define TLV( inst, par, do_tlv ) tlv_t *par;
diff --git a/src/smpp34_unpack.c b/src/smpp34_unpack.c
index 3d8b0f8..749a037 100644
--- a/src/smpp34_unpack.c
+++ b/src/smpp34_unpack.c
@@ -143,8 +143,7 @@
     };\
 }
 
-#define OCTET8( inst, par, size ){\
-    lenval = *((inst par) - 1);\
+#define OCTET8( inst, par, size, lenval ){\
     if( lenval > left ){\
         PUTLOG("[leng %s:%d(%s)]", par, lenval,\
                                       "Value length exceed buffer length");\

-- 
To view, visit https://gerrit.osmocom.org/3969
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id110f4e977c3becdb44cf5492c372e530ea51551
Gerrit-PatchSet: 1
Gerrit-Project: libsmpp34
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>



More information about the gerrit-log mailing list