rtp_engine: rtcp_report_to_json can overflow the ssrc integer value

When writing an RTCP report to json the code attempts to pack the "ssrc" and
"source_ssrc" unsigned integer values as a signed int value type. This of course
means if the ssrc's unsigned value is greater than that which can fit into a
signed integer value it gets converted to a negative number. Subsequently, the
negative value goes out in the json report.

This patch now packs the value as a json_int_t, which is the widest integer type
available on a given system. This should make it so the value no longer
overflows.

Note, this was caught by two failing tests hep/rtcp-receiver/ and
hep/rtcp-sender.

Change-Id: I2af275286ee5e795b79f0c3d450d9e4b28e958b0

diff --git a/configure b/configure
index c45e1b8c5c92e8acf11cda795328926375578bb4..ed4a049d715dc8e0ebd84f26fe5755b183bb08e3 100755 (executable)
--- a/configure
+++ b/configure
@@ -1351,7 +1351,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -1542,7 +1541,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1795,15 +1793,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1941,7 +1930,7 @@ fi
 for ac_var in  exec_prefix prefix bindir sbindir libexecdir datarootdir \
                datadir sysconfdir sharedstatedir localstatedir includedir \
                oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-               libdir localedir mandir runstatedir
+               libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -2094,7 +2083,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
@@ -9319,6 +9307,38 @@ $as_echo_n "checking for bundled jansson... " >&6; }
        JANSSON_LIB="-L${JANSSON_DIR}/dest/lib -ljansson"
        PBX_JANSSON=1
 
+       # We haven't run install yet
+
+       # Define the ast_json_int_t (large integer type) to match jansson's
+       saved_cppflags="${CPPFLAGS}"
+       CPPFLAGS="${CPPFLAGS} ${JANSSON_INCLUDE}"
+       cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <$JANSSON_DIR/source/src/jansson.h>
+int
+main ()
+{
+#if !JSON_INTEGER_IS_LONG_LONG
+               #error "not long long"
+               #endif
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+$as_echo "#define AST_JSON_INT_T long long" >>confdefs.h
+
+else
+
+$as_echo "#define AST_JSON_INT_T long" >>confdefs.h
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+       CPPFLAGS="${saved_cppflags}"
+
+
 
 
 
@@ -14718,6 +14738,36 @@ $as_echo "$as_me: *** Please install the 'libjansson' development package or" >&
 $as_echo "$as_me: *** use './configure --with-jansson-bundled'" >&6;}
                exit 1
        fi
+
+       # Define the ast_json_int_t (large integer type) to match jansson's
+       saved_cppflags="${CPPFLAGS}"
+       CPPFLAGS="${CPPFLAGS} ${JANSSON_INCLUDE}"
+       cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#include <jansson.h>
+int
+main ()
+{
+#if !JSON_INTEGER_IS_LONG_LONG
+               #error "not long long"
+               #endif
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+$as_echo "#define AST_JSON_INT_T long long" >>confdefs.h
+
+else
+
+$as_echo "#define AST_JSON_INT_T long" >>confdefs.h
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+       CPPFLAGS="${saved_cppflags}"
+
 else
        PBX_JANSSON=1
 fi
@@ -15335,7 +15385,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
                       && LARGE_OFF_T % 2147483647 == 1)
                      ? 1 : -1];
@@ -15381,7 +15431,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
                       && LARGE_OFF_T % 2147483647 == 1)
                      ? 1 : -1];
@@ -15405,7 +15455,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
                       && LARGE_OFF_T % 2147483647 == 1)
                      ? 1 : -1];
@@ -15450,7 +15500,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
                       && LARGE_OFF_T % 2147483647 == 1)
                      ? 1 : -1];
@@ -15474,7 +15524,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
+#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
                       && LARGE_OFF_T % 2147483647 == 1)
                      ? 1 : -1];
@@ -16774,8 +16824,6 @@ main ()
     if (*(data + i) != *(data3 + i))
       return 14;
   close (fd);
-  free (data);
-  free (data3);
   return 0;
 }
 _ACEOF

diff --git a/configure.ac b/configure.ac
index 69b198a8ba66c70e17157c2276227c0525767e70..b8478ca7d66ea9cd01ad5fc0047a90515abbb57a 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -734,6 +734,7 @@ if test "$JANSSON_BUNDLED" = "no" ; then
                AC_MSG_NOTICE(*** use './configure --with-jansson-bundled')
                exit 1
        fi
+       JANSSON_DEFINE_JSON_INT()
 else
        PBX_JANSSON=1
 fi

diff --git a/include/asterisk/autoconfig.h.in b/include/asterisk/autoconfig.h.in
index c90f425a2fcc9909dcb08c4adb61ef0f0ab31fb9..c2587b6317447ef27046f34b31205c086a4243f2 100644 (file)
--- a/include/asterisk/autoconfig.h.in
+++ b/include/asterisk/autoconfig.h.in
@@ -6,6 +6,9 @@
 
 #include "asterisk/buildopts.h" /* IWYU pragma: export */
 
+/* Define to 'long' or 'long long' */
+#undef AST_JSON_INT_T
+
 /* Define to 1 if internal poll should be used. */
 #undef AST_POLL_COMPAT
 

diff --git a/include/asterisk/json.h b/include/asterisk/json.h
index bd6ba86b98a3a687ff9be58e5b8f697b5ae5c971..8348a0b973a327e98801371ff80b6db2a0236b2d 100644 (file)
--- a/include/asterisk/json.h
+++ b/include/asterisk/json.h
@@ -110,6 +110,11 @@
 
 /*!@{*/
 
+/*!
+ * \brief Primarily used to cast when packing to an "I" type.
+ */
+typedef AST_JSON_INT_T ast_json_int_t;
+
 /*!
  * \brief Initialize the JSON library.
  */

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 5a7ec824db8f14c6e9a973344352bf6f39cafecd..26f8d926d273d4b3c9ade8579be91032fba7ff8c 100644 (file)
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -3373,8 +3373,8 @@ static struct ast_json *rtcp_report_to_json(struct stasis_message *msg,
                char str_lsr[32];
 
                snprintf(str_lsr, sizeof(str_lsr), "%u", payload->report->report_block[i]->lsr);
-               json_report_block = ast_json_pack("{s: i, s: i, s: i, s: i, s: i, s: s, s: i}",
-                       "source_ssrc", payload->report->report_block[i]->source_ssrc,
+               json_report_block = ast_json_pack("{s: I, s: i, s: i, s: i, s: i, s: s, s: i}",
+                       "source_ssrc", (ast_json_int_t)payload->report->report_block[i]->source_ssrc,
                        "fraction_lost", payload->report->report_block[i]->lost_count.fraction,
                        "packets_lost", payload->report->report_block[i]->lost_count.packets,
                        "highest_seq_no", payload->report->report_block[i]->highest_seq_no,
@@ -3406,8 +3406,8 @@ static struct ast_json *rtcp_report_to_json(struct stasis_message *msg,
                }
        }
 
-       json_rtcp_report = ast_json_pack("{s: i, s: i, s: i, s: o, s: o}",
-               "ssrc", payload->report->ssrc,
+       json_rtcp_report = ast_json_pack("{s: I, s: i, s: i, s: o, s: o}",
+               "ssrc", (ast_json_int_t)payload->report->ssrc,
                "type", payload->report->type,
                "report_count", payload->report->reception_report_count,
                "sender_information", json_rtcp_sender_info ?: ast_json_null(),

diff --git a/menuselect/configure b/menuselect/configure
index fd7d24b87be6a9675003ce103857f453872c588b..8efb6372e82920e98f39891d12b5d635668c224a 100755 (executable)
--- a/menuselect/configure
+++ b/menuselect/configure
@@ -692,7 +692,6 @@ infodir
 docdir
 oldincludedir
 includedir
-runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -773,7 +772,6 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE}'
@@ -1026,15 +1024,6 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
-  -runstatedir | --runstatedir | --runstatedi | --runstated \
-  | --runstate | --runstat | --runsta | --runst | --runs \
-  | --run | --ru | --r)
-    ac_prev=runstatedir ;;
-  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
-  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
-  | --run=* | --ru=* | --r=*)
-    runstatedir=$ac_optarg ;;
-
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1172,7 +1161,7 @@ fi
 for ac_var in  exec_prefix prefix bindir sbindir libexecdir datarootdir \
                datadir sysconfdir sharedstatedir localstatedir includedir \
                oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-               libdir localedir mandir runstatedir
+               libdir localedir mandir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1325,7 +1314,6 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
-  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]

diff --git a/third-party/jansson/configure.m4 b/third-party/jansson/configure.m4
index d59d8612578dac9c105d254712bb36200e7b8974..ec34076253330fffeb46283387c80aa781414c54 100644 (file)
--- a/third-party/jansson/configure.m4
+++ b/third-party/jansson/configure.m4
@@ -74,6 +74,9 @@ AC_DEFUN([_JANSSON_CONFIGURE],
        JANSSON_LIB="-L${JANSSON_DIR}/dest/lib -ljansson"
        PBX_JANSSON=1
 
+       # We haven't run install yet
+       JANSSON_DEFINE_JSON_INT([$JANSSON_DIR]/source/src/)
+
        AC_SUBST([JANSSON_BUNDLED])
        AC_SUBST([PBX_JANSSON])
        AC_SUBST([JANSSON_LIB])
@@ -87,3 +90,19 @@ AC_DEFUN([JANSSON_CONFIGURE],
                _JANSSON_CONFIGURE()
        fi
 ])
+
+AC_DEFUN([JANSSON_DEFINE_JSON_INT],
+[
+       # Define the ast_json_int_t (large integer type) to match jansson's
+       saved_cppflags="${CPPFLAGS}"
+       CPPFLAGS="${CPPFLAGS} ${JANSSON_INCLUDE}"
+       AC_COMPILE_IFELSE(
+               [AC_LANG_PROGRAM([#include <$1jansson.h>],
+               [#if !JSON_INTEGER_IS_LONG_LONG
+               #error "not long long"
+               #endif
+               ])],
+               [AC_DEFINE([AST_JSON_INT_T], [long long], [Define to 'long' or 'long long'])],
+               [AC_DEFINE([AST_JSON_INT_T], [long], [Define to 'long' or 'long long'])])
+       CPPFLAGS="${saved_cppflags}"
+])