[Asterisk-code-review] rtp engine: rtcp report to json can overflow the ssrc intege... (asterisk[master])
George Joseph
asteriskteam at digium.com
Wed Sep 26 08:02:29 CDT 2018
George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/10151 )
Change subject: rtp_engine: rtcp_report_to_json can overflow the ssrc integer value
......................................................................
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
---
M configure
M configure.ac
M include/asterisk/autoconfig.h.in
M include/asterisk/json.h
M main/rtp_engine.c
M menuselect/configure
M third-party/jansson/configure.m4
7 files changed, 101 insertions(+), 37 deletions(-)
Approvals:
Corey Farrell: Looks good to me, but someone else must approve
Richard Mudgett: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/configure b/configure
index 6c5d973..7f05720 100755
--- a/configure
+++ b/configure
@@ -1337,7 +1337,6 @@
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -1525,7 +1524,6 @@
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -1778,15 +1776,6 @@
| -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=* \
@@ -1924,7 +1913,7 @@
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.
@@ -2077,7 +2066,6 @@
--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]
@@ -9298,6 +9286,38 @@
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}"
+
+
@@ -14198,6 +14218,36 @@
$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
@@ -14815,7 +14865,7 @@
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];
@@ -14861,7 +14911,7 @@
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];
@@ -14885,7 +14935,7 @@
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];
@@ -14930,7 +14980,7 @@
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];
@@ -14954,7 +15004,7 @@
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];
@@ -16254,8 +16304,6 @@
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 7efbec1..940bc18 100644
--- a/configure.ac
+++ b/configure.ac
@@ -688,6 +688,7 @@
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 69ab585..0291741 100644
--- 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 3da7165..227afbb 100644
--- a/include/asterisk/json.h
+++ b/include/asterisk/json.h
@@ -82,6 +82,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.
*/
void ast_json_init(void);
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 3d50774..0113144 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -3379,8 +3379,8 @@
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,
@@ -3412,8 +3412,8 @@
}
}
- 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 fd7d24b..8efb637 100755
--- a/menuselect/configure
+++ b/menuselect/configure
@@ -692,7 +692,6 @@
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -773,7 +772,6 @@
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 @@
| -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 @@
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 @@
--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 d59d861..ec34076 100644
--- a/third-party/jansson/configure.m4
+++ b/third-party/jansson/configure.m4
@@ -74,6 +74,9 @@
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 @@
_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}"
+])
--
To view, visit https://gerrit.asterisk.org/10151
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2af275286ee5e795b79f0c3d450d9e4b28e958b0
Gerrit-Change-Number: 10151
Gerrit-PatchSet: 5
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180926/55f0acc5/attachment-0001.html>
More information about the asterisk-code-review
mailing list