Bug 844929: Accept numeric parameters > UINT_MAX in SDP o-lines
authorEthan Hugg <ethanhugg@gmail.com>
Mon, 25 Feb 2013 10:22:10 -0800
changeset 123067 a1d01526d34421e59928bd4e9babd3e5bdb46a25
parent 123066 036f8a67636cc709aa2dccaab8495316bdc6f433
child 123068 00ed3d264438713fe8450d6c713b1f2040ee33ea
push id24372
push useremorley@mozilla.com
push dateWed, 27 Feb 2013 13:22:59 +0000
treeherdermozilla-central@0a91da5f5eab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs844929
milestone22.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 844929: Accept numeric parameters > UINT_MAX in SDP o-lines
media/webrtc/signaling/src/sipcc/core/sdp/sdp_token.c
media/webrtc/signaling/test/signaling_unittests.cpp
--- a/media/webrtc/signaling/src/sipcc/core/sdp/sdp_token.c
+++ b/media/webrtc/signaling/src/sipcc/core/sdp/sdp_token.c
@@ -6,16 +6,17 @@
 
 #include "sdp_os_defs.h"
 #include "sdp.h"
 #include "sdp_private.h"
 #include "configmgr.h"
 #include "prot_configmgr.h"
 #include "ccapi.h"
 #include "CSFLog.h"
+#include "prprf.h"
 
 static const char *logTag = "sdp_token";
 
 #define MCAST_STRING_LEN 4
 
 
 sdp_result_e sdp_parse_version (sdp_t *sdp_p, u16 level, const char *ptr)
 {
@@ -54,22 +55,49 @@ sdp_result_e sdp_build_version (sdp_t *s
     flex_string_sprintf(fs, "v=%u\r\n", (u16)sdp_p->version);
 
     if (sdp_p->debug_flag[SDP_DEBUG_TRACE]) {
         SDP_PRINT("%s Built v= version line", sdp_p->debug_str);
     }
     return (SDP_SUCCESS);
 }
 
+static sdp_result_e sdp_verify_unsigned(const char *ptr, PRUint64 max_value)
+{
+    PRUint64 numeric_value;
+    /* Checking for only numbers since PR_sscanf will ignore trailing
+       characters */
+    size_t end = strspn(ptr, "0123456789");
+
+    if (ptr[end] != '\0')
+        return SDP_INVALID_PARAMETER;
+
+    if (PR_sscanf(ptr, "%llu", &numeric_value) != 1)
+        return SDP_INVALID_PARAMETER;
+
+    if (numeric_value > max_value)
+        return SDP_INVALID_PARAMETER;
+
+    return SDP_SUCCESS;
+}
+
 sdp_result_e sdp_parse_owner (sdp_t *sdp_p, u16 level, const char *ptr)
 {
     int          i;
     char        *tmpptr;
     sdp_result_e result;
     char         tmp[SDP_MAX_STRING_LEN];
+    /* The spec says this:
+
+        The numeric value of the session id
+        and version in the o line MUST be representable with a 64 bit signed
+        integer.  The initial value of the version MUST be less than
+        (2**62)-1, to avoid rollovers.
+    */
+    PRUint64     max_value_sessid_version = ((((PRUint64) 1) << 62) - 2);
 
     if (sdp_p->owner_name[0] != '\0') {
         sdp_p->conf_p->num_invalid_token_order++;
         sdp_parse_error(sdp_p->peerconnection,
             "%s Warning: More than one o= line specified.",
             sdp_p->debug_str);
     }
 
@@ -86,35 +114,33 @@ sdp_result_e sdp_parse_owner (sdp_t *sdp
     /* Find the owner session id.  This is a numeric field but is
      * stored as a string since it may be 64 bit.
      */
     ptr = sdp_getnextstrtok(ptr, sdp_p->owner_sessid, sizeof(sdp_p->owner_sessid), " \t", &result);
     if (result == SDP_SUCCESS) {
         /* Make sure the sessid is numeric, even though we store it as
          * a string.
          */
-        (void)sdp_getnextnumtok(sdp_p->owner_sessid,
-                                (const char **)&tmpptr, " \t",&result);
+        result = sdp_verify_unsigned(sdp_p->owner_sessid, max_value_sessid_version);
     }
     if (result != SDP_SUCCESS) {
         sdp_parse_error(sdp_p->peerconnection,
             "%s Invalid owner session id specified for o=.",
             sdp_p->debug_str);
         sdp_p->conf_p->num_invalid_param++;
         return (SDP_INVALID_PARAMETER);
     }
 
     /* Find the owner version. */
     ptr = sdp_getnextstrtok(ptr, sdp_p->owner_version, sizeof(sdp_p->owner_version), " \t", &result);
     if (result == SDP_SUCCESS) {
         /* Make sure the version is numeric, even though we store it as
          * a string.
          */
-        (void)sdp_getnextnumtok(sdp_p->owner_version,
-                                (const char **)&tmpptr," \t",&result);
+        result = sdp_verify_unsigned(sdp_p->owner_version, max_value_sessid_version);
     }
     if (result != SDP_SUCCESS) {
         sdp_parse_error(sdp_p->peerconnection,
             "%s Invalid owner version specified for o=.",
             sdp_p->debug_str);
         sdp_p->conf_p->num_invalid_param++;
         return (SDP_INVALID_PARAMETER);
     }
--- a/media/webrtc/signaling/test/signaling_unittests.cpp
+++ b/media/webrtc/signaling/test/signaling_unittests.cpp
@@ -1923,16 +1923,67 @@ TEST_F(SignalingTest, ipAddrAnyOffer)
     a2_.SetRemote(TestObserver::OFFER, offer);
     ASSERT_TRUE(a2_.pObserver->state == TestObserver::stateSuccess);
     a2_.CreateAnswer(constraints, offer, OFFER_AUDIO | ANSWER_AUDIO);
     ASSERT_TRUE(a2_.pObserver->state == TestObserver::stateSuccess);
     std::string answer = a2_.answer();
     ASSERT_NE(answer.find("a=sendrecv"), std::string::npos);
 }
 
+static void CreateSDPForBigOTests(std::string& offer, const char *number) {
+  offer =
+    "v=0\r\n"
+    "o=- ";
+  offer += number;
+  offer += " ";
+  offer += number;
+  offer += " IN IP4 127.0.0.1\r\n"
+    "s=-\r\n"
+    "b=AS:64\r\n"
+    "t=0 0\r\n"
+    "a=fingerprint:sha-256 F3:FA:20:C0:CD:48:C4:5F:02:5F:A5:D3:21:D0:2D:48:"
+      "7B:31:60:5C:5A:D8:0D:CD:78:78:6C:6D:CE:CC:0C:67\r\n"
+    "m=audio 9000 RTP/AVP 99\r\n"
+    "c=IN IP4 0.0.0.0\r\n"
+    "a=rtpmap:99 opus/48000/2\r\n"
+    "a=ice-ufrag:cYuakxkEKH+RApYE\r\n"
+    "a=ice-pwd:bwtpzLZD+3jbu8vQHvEa6Xuq\r\n"
+    "a=sendrecv\r\n";
+}
+
+TEST_F(SignalingTest, BigOValues)
+{
+  std::string offer;
+
+  CreateSDPForBigOTests(offer, "12345678901234567");
+
+  a2_.SetRemote(TestObserver::OFFER, offer);
+  ASSERT_TRUE(a2_.pObserver->state == TestObserver::stateSuccess);
+}
+
+TEST_F(SignalingTest, BigOValuesExtraChars)
+{
+  std::string offer;
+
+  CreateSDPForBigOTests(offer, "12345678901234567FOOBAR");
+
+  a2_.SetRemote(TestObserver::OFFER, offer, true);
+  ASSERT_TRUE(a2_.pObserver->state == TestObserver::stateError);
+}
+
+TEST_F(SignalingTest, BigOValuesTooBig)
+{
+  std::string offer;
+
+  CreateSDPForBigOTests(offer, "18446744073709551615");
+
+  a2_.SetRemote(TestObserver::OFFER, offer, true);
+  ASSERT_TRUE(a2_.pObserver->state == TestObserver::stateError);
+}
+
 TEST_F(SignalingAgentTest, CreateUntilFailThenWait) {
   int i;
 
   for (i=0; ; i++) {
     if (!CreateAgent())
       break;
     std::cerr << "Created agent " << i << std::endl;
   }