[asterisk-commits] mjordan: branch certified-11.2 r383974 - in /certified/branches/11.2: ./ res/

[SVN commits to the Asterisk project]

Author: mjordan
Date: Wed Mar 27 09:27:43 2013
New Revision: 383974

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=383974
Log:
AST-2013-001: Prevent buffer overflow through H.264 format negotiation

The format attribute resource for H.264 video performs an unsafe read against a
media attribute when parsing the SDP. The value passed in with the format
attribute is not checked for its length when parsed into a fixed length buffer.
This patch resolves the vulnerability by only reading as many characters from
the SDP value as will fit into the buffer.

(closes issue ASTERISK-20901)
Reported by: Ulf Harnhammar
patches:
  h264_overflow_security_patch.diff uploaded by jrose (License 6182)
........

Merged revisions 383973 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    certified/branches/11.2/   (props changed)
    certified/branches/11.2/res/res_format_attr_h264.c

Propchange: certified/branches/11.2/
------------------------------------------------------------------------------
--- branch-11-merged (original)
+++ branch-11-merged Wed Mar 27 09:27:43 2013
@@ -1,1 +1,1 @@
-/branches/11:378038,378121,378287,378321,378409-378411,378459,378582,378687,378690,378984,379513,379790,380465,380698,380869,380892,380894,380974,381306,381594,381613,381702,381737,382385,382390,382573,382617,383166,383840,383878
+/branches/11:378038,378121,378287,378321,378409-378411,378459,378582,378687,378690,378984,379513,379790,380465,380698,380869,380892,380894,380974,381306,381594,381613,381702,381737,382385,382390,382573,382617,383166,383840,383878,383973

Modified: certified/branches/11.2/res/res_format_attr_h264.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/11.2/res/res_format_attr_h264.c?view=diff&rev=383974&r1=383973&r2=383974
==============================================================================
--- certified/branches/11.2/res/res_format_attr_h264.c (original)
+++ certified/branches/11.2/res/res_format_attr_h264.c Wed Mar 27 09:27:43 2013
@@ -41,8 +41,14 @@
 /*! \brief Value that indicates an attribute is actually unset */
 #define H264_ATTR_KEY_UNSET UINT8_MAX
 
-/*! \brief Maximum size for SPS / PPS values in sprop-parameter-sets attribute */
+/*! \brief Maximum size for SPS / PPS values in sprop-parameter-sets attribute
+ *   if you change this value then you must change H264_MAX_SPS_PPS_SIZE_SCAN_LIMIT
+ *   as well. */
 #define H264_MAX_SPS_PPS_SIZE 16
+/*! \brief This is used when executing sscanf on buffers of H264_MAX_SPS_PPS_SIZE
+ *   length. It must ALWAYS be a string literal representation of one less than
+ *   H264_MAX_SPS_PPS_SIZE */
+#define H264_MAX_SPS_PPS_SIZE_SCAN_LIMIT "15"
 
 enum h264_attr_keys {
 	H264_ATTR_KEY_PROFILE_IDC,
@@ -111,7 +117,8 @@
 			format_attr->format_attr[H264_ATTR_KEY_PROFILE_IDC] = ((val2 >> 16) & 0xFF);
 			format_attr->format_attr[H264_ATTR_KEY_PROFILE_IOP] = ((val2 >> 8) & 0xFF);
 			format_attr->format_attr[H264_ATTR_KEY_LEVEL] = (val2 & 0xFF);
-		} else if (sscanf(attrib, "sprop-parameter-sets=%[^','],%s", sps, pps) == 2) {
+		} else if (sscanf(attrib, "sprop-parameter-sets=%" H264_MAX_SPS_PPS_SIZE_SCAN_LIMIT "[^','],%" H264_MAX_SPS_PPS_SIZE_SCAN_LIMIT "s", sps, pps) == 2) {
+			/* XXX sprop-parameter-sets can actually be of unlimited length. This may need to be addressed later. */
 			unsigned char spsdecoded[H264_MAX_SPS_PPS_SIZE] = { 0, }, ppsdecoded[H264_MAX_SPS_PPS_SIZE] = { 0, };
 			int i;