From 82f9a77777fa1897e0bafb2a1265835720a27050 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 12 Apr 2020 08:32:34 +0200 Subject: [PATCH] be more conservative when parsing rtp content --- .../xmpp/jingle/SessionDescription.java | 20 ++++++++++++++++++- .../xmpp/jingle/stanzas/RtpDescription.java | 15 ++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java index f6f12513d..62424e81f 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java @@ -3,6 +3,7 @@ package eu.siacs.conversations.xmpp.jingle; import android.util.Log; import android.util.Pair; +import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ArrayListMultimap; @@ -131,6 +132,8 @@ public class SessionDescription { final ImmutableList.Builder mediaListBuilder = new ImmutableList.Builder<>(); final Group group = contentMap.group; if (group != null) { + final String semantics = group.getSemantics(); + checkNoWhitespace(semantics, "group semantics value must not contain any whitespace"); attributeMap.put("group", group.getSemantics() + " " + Joiner.on(' ').join(group.getIdentificationTags())); } @@ -150,9 +153,11 @@ public class SessionDescription { if (!Strings.isNullOrEmpty(ufrag)) { mediaAttributes.put("ice-ufrag", ufrag); } + checkNoWhitespace(ufrag, "ufrag value must not contain any whitespaces"); if (!Strings.isNullOrEmpty(pwd)) { mediaAttributes.put("ice-pwd", pwd); } + checkNoWhitespace(pwd, "pwd value must not contain any whitespaces"); mediaAttributes.put("ice-options", HARDCODED_ICE_OPTIONS); final IceUdpTransportInfo.Fingerprint fingerprint = transport.getFingerprint(); if (fingerprint != null) { @@ -180,6 +185,7 @@ public class SessionDescription { if (Strings.isNullOrEmpty(type)) { throw new IllegalArgumentException("a feedback for payload-type " + id + " negotiation is missing type"); } + checkNoWhitespace(type, "feedback negotiation type must not contain whitespace"); mediaAttributes.put("rtcp-fb", id + " " + type + (Strings.isNullOrEmpty(subtype) ? "" : " " + subtype)); } for (RtpDescription.FeedbackNegotiationTrrInt feedbackNegotiationTrrInt : payloadType.feedbackNegotiationTrrInts()) { @@ -193,6 +199,7 @@ public class SessionDescription { if (Strings.isNullOrEmpty(type)) { throw new IllegalArgumentException("a feedback negotiation is missing type"); } + checkNoWhitespace(type, "feedback negotiation type must not contain whitespace"); mediaAttributes.put("rtcp-fb", "* " + type + (Strings.isNullOrEmpty(subtype) ? "" : " " + subtype)); } for (RtpDescription.FeedbackNegotiationTrrInt feedbackNegotiationTrrInt : description.feedbackNegotiationTrrInts()) { @@ -204,9 +211,11 @@ public class SessionDescription { if (Strings.isNullOrEmpty(id)) { throw new IllegalArgumentException("A header extension is missing id"); } + checkNoWhitespace(id, "header extension id must not contain whitespace"); if (Strings.isNullOrEmpty(uri)) { throw new IllegalArgumentException("A header extension is missing uri"); } + checkNoWhitespace(uri, "feedback negotiation uri must not contain whitespace"); mediaAttributes.put("extmap", id + " " + uri); } for (RtpDescription.SourceGroup sourceGroup : description.getSourceGroups()) { @@ -215,6 +224,7 @@ public class SessionDescription { if (Strings.isNullOrEmpty(semantics)) { throw new IllegalArgumentException("A SSRC group is missing semantics attribute"); } + checkNoWhitespace(semantics, "source group semantics must not contain whitespace"); if (groups.size() == 0) { throw new IllegalArgumentException("A SSRC group is missing SSRC ids"); } @@ -228,13 +238,14 @@ public class SessionDescription { if (Strings.isNullOrEmpty(id)) { throw new IllegalArgumentException("A source specific media attribute is missing the id"); } + checkNoWhitespace(id, "A source specific media attributes must not contain whitespaces"); if (Strings.isNullOrEmpty(parameterName)) { throw new IllegalArgumentException("A source specific media attribute is missing its name"); } if (Strings.isNullOrEmpty(parameterValue)) { throw new IllegalArgumentException("A source specific media attribute is missing its value"); } - mediaAttributes.put("ssrc", id + " " + parameter.getParameterName() + ":" + parameter.getParameterValue()); + mediaAttributes.put("ssrc", id + " " + parameterName + ":" + parameterValue); } } @@ -263,6 +274,13 @@ public class SessionDescription { return sessionDescriptionBuilder.createSessionDescription(); } + public static String checkNoWhitespace(final String input, final String message) { + if (CharMatcher.whitespace().matchesAnyOf(input)) { + throw new IllegalArgumentException(message); + } + return input; + } + public static int ignorantIntParser(final String input) { try { return Integer.parseInt(input); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java index 00b2ddc60..5f1a76af9 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java @@ -232,7 +232,10 @@ public class RtpDescription extends GenericDescription { public String toSdpAttribute() { final int channels = getChannels(); - return getId()+" "+getPayloadTypeName()+"/"+getClockRate()+(channels == 1 ? "" : "/"+channels); + final String name = getPayloadTypeName(); + Preconditions.checkArgument(name != null, "Payload-type name must not be empty"); + SessionDescription.checkNoWhitespace(name, "payload-type name must not contain whitespaces"); + return getId()+" "+name+"/"+getClockRate()+(channels == 1 ? "" : "/"+channels); } public int getIntId() { @@ -367,7 +370,15 @@ public class RtpDescription extends GenericDescription { stringBuilder.append(id).append(' '); for(int i = 0; i < parameters.size(); ++i) { Parameter p = parameters.get(i); - stringBuilder.append(p.getParameterName()).append('=').append(p.getParameterValue()); + final String name = p.getParameterName(); + Preconditions.checkArgument(name != null, String.format("parameter for %s must have a name", id)); + SessionDescription.checkNoWhitespace(name, String.format("parameter names for %s must not contain whitespaces", id)); + + final String value = p.getParameterValue(); + Preconditions.checkArgument(value != null, String.format("parameter for %s must have a value", id)); + SessionDescription.checkNoWhitespace(value, String.format("parameter values for %s must not contain whitespaces", id)); + + stringBuilder.append(name).append('=').append(value); if (i != parameters.size() - 1) { stringBuilder.append(';'); }