From b8df6cc316f76eda5f592a70a7d0a69cf6eeb9f8 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Tue, 29 Sep 2015 16:17:03 +0100 Subject: [PATCH] mpdparser: validate representation set identifier It must have no whitespace, and must comply with RFC 1738 when used to build a URL. https://bugzilla.gnome.org/show_bug.cgi?id=750852 --- ext/dash/gstmpdparser.c | 63 +++++++++++++++++++++++++++++++-- tests/check/elements/dash_mpd.c | 47 ++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 5 deletions(-) diff --git a/ext/dash/gstmpdparser.c b/ext/dash/gstmpdparser.c index 6973e7cf43..07532f15f1 100644 --- a/ext/dash/gstmpdparser.c +++ b/ext/dash/gstmpdparser.c @@ -33,6 +33,9 @@ #define GST_CAT_DEFAULT gst_dash_demux_debug /* Property parsing */ +static gboolean gst_mpdparser_get_xml_prop_validated_string (xmlNode * a_node, + const gchar * property_name, gchar ** property_value, + gboolean (*validator) (const char *)); static gboolean gst_mpdparser_get_xml_prop_string (xmlNode * a_node, const gchar * property_name, gchar ** property_value); static gboolean gst_mpdparser_get_xml_ns_prop_string (xmlNode * a_node, @@ -253,14 +256,20 @@ static const struct GstMpdParserUtcTimingMethod /* functions to parse node namespaces, content and properties */ static gboolean -gst_mpdparser_get_xml_prop_string (xmlNode * a_node, - const gchar * property_name, gchar ** property_value) +gst_mpdparser_get_xml_prop_validated_string (xmlNode * a_node, + const gchar * property_name, gchar ** property_value, + gboolean (*validate) (const char *)) { xmlChar *prop_string; gboolean exists = FALSE; prop_string = xmlGetProp (a_node, (const xmlChar *) property_name); if (prop_string) { + if (validate && !(*validate) ((const char *) prop_string)) { + GST_WARNING ("Validation failure: %s", prop_string); + xmlFree (prop_string); + return FALSE; + } *property_value = (gchar *) prop_string; exists = TRUE; GST_LOG (" - %s: %s", property_name, prop_string); @@ -288,6 +297,28 @@ gst_mpdparser_get_xml_ns_prop_string (xmlNode * a_node, return exists; } +static gboolean +gst_mpdparser_get_xml_prop_string (xmlNode * a_node, + const gchar * property_name, gchar ** property_value) +{ + return gst_mpdparser_get_xml_prop_validated_string (a_node, property_name, + property_value, NULL); +} + +static gboolean +gst_mpdparser_validate_no_whitespace (const char *s) +{ + return !strpbrk (s, "\r\n\t "); +} + +static gboolean +gst_mpdparser_get_xml_prop_string_no_whitespace (xmlNode * a_node, + const gchar * property_name, gchar ** property_value) +{ + return gst_mpdparser_get_xml_prop_validated_string (a_node, property_name, + property_value, gst_mpdparser_validate_no_whitespace); +} + static gboolean gst_mpdparser_get_xml_prop_string_vector_type (xmlNode * a_node, const gchar * property_name, gchar *** property_value) @@ -1571,7 +1602,8 @@ gst_mpdparser_parse_representation_node (GList ** list, xmlNode * a_node, *list = g_list_append (*list, new_representation); GST_LOG ("attributes of Representation node:"); - gst_mpdparser_get_xml_prop_string (a_node, "id", &new_representation->id); + gst_mpdparser_get_xml_prop_string_no_whitespace (a_node, "id", + &new_representation->id); gst_mpdparser_get_xml_prop_unsigned_integer (a_node, "bandwidth", 0, &new_representation->bandwidth); gst_mpdparser_get_xml_prop_unsigned_integer (a_node, "qualityRanking", 0, @@ -2868,6 +2900,26 @@ promote_format_to_uint64 (const gchar * format) return promoted_format; } +static gboolean +gst_mpdparser_validate_rfc1738_url (const char *s) +{ + while (*s) { + if (!strchr + (";:@&=aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ0123456789$-_.+!*'(),%", + *s)) + return FALSE; + if (*s == '%') { + /* g_ascii_isdigit returns FALSE for NUL, and || is a short circuiting + operator, so this is safe for strings ending before two hex digits */ + if (!g_ascii_isxdigit (s[1]) || !g_ascii_isxdigit (s[2])) + return FALSE; + s += 2; + } + s++; + } + return TRUE; +} + static gchar * gst_mpdparser_build_URL_from_template (const gchar * url_template, const gchar * id, guint number, guint bandwidth, guint64 time) @@ -2891,6 +2943,11 @@ gst_mpdparser_build_URL_from_template (const gchar * url_template, format = default_format; if (!g_strcmp0 (token, "RepresentationID")) { + if (!gst_mpdparser_validate_rfc1738_url (id)) { + GST_WARNING ("String '%s' has characters invalid in an RFC 1738 URL", + id); + goto invalid_format; + } tokens[i] = g_strdup_printf ("%s", id); g_free (token); last_token_par = TRUE; diff --git a/tests/check/elements/dash_mpd.c b/tests/check/elements/dash_mpd.c index 23822ce6b5..c48bc9a14b 100644 --- a/tests/check/elements/dash_mpd.c +++ b/tests/check/elements/dash_mpd.c @@ -2016,7 +2016,7 @@ GST_START_TEST (dash_mpdparser_period_adaptationSet_representation) " profiles=\"urn:mpeg:dash:profile:isoff-main:2011\">" " " " " - " AdaptationSets->data; representation = (GstRepresentationNode *) adaptationSet->Representations->data; - assert_equals_string (representation->id, "Test Id"); + assert_equals_string (representation->id, "Test_Id"); assert_equals_uint64 (representation->bandwidth, 100); assert_equals_uint64 (representation->qualityRanking, 200); assert_equals_string (representation->dependencyId[0], "one"); @@ -4447,6 +4447,44 @@ GST_START_TEST GST_END_TEST; +GST_START_TEST (dash_mpdparser_whitespace_strings) +{ + fail_unless (gst_mpdparser_validate_no_whitespace ("") == TRUE); + fail_unless (gst_mpdparser_validate_no_whitespace ("/") == TRUE); + fail_unless (gst_mpdparser_validate_no_whitespace (" ") == FALSE); + fail_unless (gst_mpdparser_validate_no_whitespace ("aaaaaaaa ") == FALSE); + fail_unless (gst_mpdparser_validate_no_whitespace ("a\ta") == FALSE); + fail_unless (gst_mpdparser_validate_no_whitespace ("a\ra") == FALSE); + fail_unless (gst_mpdparser_validate_no_whitespace ("a\na") == FALSE); +} + +GST_END_TEST; + +GST_START_TEST (dash_mpdparser_rfc1738_strings) +{ + fail_unless (gst_mpdparser_validate_rfc1738_url ("/") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url (" ") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("aaaaaaaa ") == FALSE); + + fail_unless (gst_mpdparser_validate_rfc1738_url ("") == TRUE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("a") == TRUE); + fail_unless (gst_mpdparser_validate_rfc1738_url + (";:@&=aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ0123456789$-_.+!*'(),%AA") + == TRUE); + fail_unless (gst_mpdparser_validate_rfc1738_url + (";:@&=aAbBcCdDeEfFgGhHiIjJkKlLmMnNoOpPqQrRsStTuUvVwWxXyYzZ0123456789$-_.+!*'(),% ") + == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("%AA") == TRUE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("%A") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("%") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("%XA") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("%AX") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("%XX") == FALSE); + fail_unless (gst_mpdparser_validate_rfc1738_url ("\001") == FALSE); +} + +GST_END_TEST; + /* * create a test suite containing all dash testcases */ @@ -4457,6 +4495,7 @@ dash_suite (void) TCase *tc_simpleMPD = tcase_create ("simpleMPD"); TCase *tc_complexMPD = tcase_create ("complexMPD"); TCase *tc_negativeTests = tcase_create ("negativeTests"); + TCase *tc_stringTests = tcase_create ("stringTests"); GST_DEBUG_CATEGORY_INIT (gst_dash_demux_debug, "gst_dash_demux_debug", 0, "mpeg dash tests"); @@ -4605,9 +4644,13 @@ dash_suite (void) tcase_add_test (tc_negativeTests, dash_mpdparser_wrong_period_duration_inferred_from_next_mediaPresentationDuration); + tcase_add_test (tc_stringTests, dash_mpdparser_whitespace_strings); + tcase_add_test (tc_stringTests, dash_mpdparser_rfc1738_strings); + suite_add_tcase (s, tc_simpleMPD); suite_add_tcase (s, tc_complexMPD); suite_add_tcase (s, tc_negativeTests); + suite_add_tcase (s, tc_stringTests); return s; }