fix: validate signaling JSON and ICE indexes strictly

docs/guide/client-api.md

@@ -127,7 +127,7 @@ A `SignalMessage` is a session description (`OFFER`, `ANSWER`) or an ICE candida
 {"candidate":"candidate:842163049 1 udp 1677729535 203.0.113.7 49203 typ srflx raddr 0.0.0.0 rport 0","sdpMid":"0","sdpMLineIndex":0}
 ```
 
-`SignalMessage.fromJson(String)` reads these, what a browser's `JSON.stringify(pc.localDescription)` and `JSON.stringify(candidate)` write, and the `{"description": ...}` and `{"candidate": {...}}` envelopes of the perfect negotiation examples. Anything else throws an `IllegalArgumentException`, as does text longer than a MiB or nested deeper than 64 levels, so a message from the remote side cannot exhaust the stack. Routing, such as room names or peer ids, belongs in an envelope of your own around this JSON.
+`SignalMessage.fromJson(String)` reads these, what a browser's `JSON.stringify(pc.localDescription)` and `JSON.stringify(candidate)` write, and the `{"description": ...}` and `{"candidate": {...}}` envelopes of the perfect negotiation examples. Anything else throws an `IllegalArgumentException` that says what is wrong and where, as does text longer than a MiB or nested deeper than 64 levels, so a message from the remote side cannot exhaust the stack. The JSON must be strict (RFC 8259): no raw line breaks or other control characters inside strings, no member name twice in one object, no numbers like `01`, `1.` or `1e999`. An `sdpMLineIndex` must be an integer from 0 to 65535; `1.9` is rejected rather than truncated to 1. Routing, such as room names or peer ids, belongs in an envelope of your own around this JSON.
 
 A browser peer that only answers needs no more than this:
 

webrtc/src/main/java/dev/onvoid/webrtc/client/Json.java

@@ -26,6 +26,12 @@
  * library. Objects become {@code Map}s, arrays {@code List}s, numbers {@code
  * Double}s.
  * <p>
+ * The grammar is that of RFC 8259 and nothing more lenient: no control
+ * characters inside strings, numbers without leading zeros or bare points,
+ * only the four whitespace characters JSON has. A member name must not appear
+ * twice in an object, which would leave open which of the values counts, and
+ * a number must fit a {@code double}, since JSON has no infinities.
+ * <p>
  * Text comes from the remote side, so it is bounded: in length, like the
  * answers of WHIP and WHEP endpoints, and in nesting, which a signaling
  * message needs only a few levels of and the recursive parser would otherwise
@@ -143,7 +149,7 @@ private Object readValue() {
 				if (c == '-' || (c >= '0' && c <= '9')) {
 					return readNumber();
 				}
-				throw error("Unexpected character '" + c + "'");
+				throw error("Unexpected character " + describe(c));
 		}
 	}
 
@@ -164,7 +170,13 @@ private Map<String, Object> readObject() {
 				throw error("Expected a member name");
 			}
 
+			int start = pos;
 			String name = readString();
+
+			if (object.containsKey(name)) {
+				throw error("Duplicate member name " + quote(name), start);
+			}
+
 			skipWhitespace();
 
 			if (peek() != ':') {
@@ -229,6 +241,9 @@ private String readString() {
 			if (c == '"') {
 				return out.toString();
 			}
+			if (c < 0x20) {
+				throw error("Unescaped control character " + describe(c) + " in a string", pos - 1);
+			}
 			if (c != '\\') {
 				out.append(c);
 				continue;
@@ -264,12 +279,20 @@ private String readString() {
 					if (pos + 4 > text.length()) {
 						throw error("Incomplete unicode escape");
 					}
-					try {
-						out.append((char) Integer.parseInt(text.substring(pos, pos + 4), 16));
-					}
-					catch (NumberFormatException ex) {
-						throw error("Invalid unicode escape");
+
+					int code = 0;
+
+					for (int i = 0; i < 4; i++) {
+						int digit = hexDigit(text.charAt(pos + i));
+
+						if (digit < 0) {
+							throw error("Invalid unicode escape", pos + i);
+						}
+
+						code = code << 4 | digit;
 					}
+
+					out.append((char) code);
 					pos += 4;
 					break;
 				default:
@@ -281,16 +304,59 @@ private String readString() {
 	private Double readNumber() {
 		int start = pos;
 
-		while (pos < text.length() && "+-0123456789.eE".indexOf(text.charAt(pos)) >= 0) {
+		if (next('-')) {
 			pos++;
 		}
+		if (next('0')) {
+			pos++;
 
-		try {
-			return Double.valueOf(text.substring(start, pos));
+			if (nextIsDigit()) {
+				throw error("Leading zero in a number");
+			}
 		}
-		catch (NumberFormatException e) {
-			throw error("Invalid number");
+		else {
+			readDigits("Expected a digit");
 		}
+		if (next('.')) {
+			pos++;
+			readDigits("Expected a digit after the decimal point");
+		}
+		if (next('e') || next('E')) {
+			pos++;
+
+			if (next('+') || next('-')) {
+				pos++;
+			}
+
+			readDigits("Expected a digit in the exponent");
+		}
+
+		// The grammar above is a subset of what Double.valueOf reads.
+		Double value = Double.valueOf(text.substring(start, pos));
+
+		if (value.isInfinite()) {
+			throw error("Number out of range", start);
+		}
+
+		return value;
+	}
+
+	private void readDigits(String message) {
+		if (!nextIsDigit()) {
+			throw error(message);
+		}
+
+		while (nextIsDigit()) {
+			pos++;
+		}
+	}
+
+	private boolean next(char c) {
+		return pos < text.length() && text.charAt(pos) == c;
+	}
+
+	private boolean nextIsDigit() {
+		return pos < text.length() && text.charAt(pos) >= '0' && text.charAt(pos) <= '9';
 	}
 
 	private void enter() {
@@ -316,12 +382,55 @@ private char peek() {
 	}
 
 	private void skipWhitespace() {
-		while (pos < text.length() && Character.isWhitespace(text.charAt(pos))) {
+		while (pos < text.length() && isWhitespace(text.charAt(pos))) {
 			pos++;
 		}
 	}
 
 	private IllegalArgumentException error(String message) {
-		return new IllegalArgumentException(message + " at position " + pos);
+		return error(message, pos);
+	}
+
+	private static IllegalArgumentException error(String message, int position) {
+		return new IllegalArgumentException(message + " at position " + position);
+	}
+
+	private static boolean isWhitespace(char c) {
+		return c == ' ' || c == '\t' || c == '\n' || c == '\r';
+	}
+
+	private static int hexDigit(char c) {
+		if (c >= '0' && c <= '9') {
+			return c - '0';
+		}
+		if (c >= 'a' && c <= 'f') {
+			return c - 'a' + 10;
+		}
+		if (c >= 'A' && c <= 'F') {
+			return c - 'A' + 10;
+		}
+
+		return -1;
+	}
+
+	/**
+	 * @return the character for a message, readable whatever it is.
+	 */
+	private static String describe(char c) {
+		if (c >= 0x20 && c < 0x7f) {
+			return "'" + c + "'";
+		}
+
+		return String.format("U+%04X", (int) c);
+	}
+
+	/**
+	 * @return the text as a JSON string for a message, cut short if long.
+	 */
+	private static String quote(String text) {
+		StringBuilder out = new StringBuilder();
+		writeString(out, text.length() > 32 ? text.substring(0, 32) + "..." : text);
+
+		return out.toString();
 	}
 }

webrtc/src/main/java/dev/onvoid/webrtc/client/SignalMessage.java

@@ -64,6 +64,12 @@ public enum Type {
 	private static final SignalMessage END_OF_CANDIDATES =
 			new SignalMessage(Type.END_OF_CANDIDATES, null, null);
 
+	/**
+	 * The largest {@code sdpMLineIndex}, which the browser API types as an
+	 * {@code unsigned short}.
+	 */
+	private static final int MAX_M_LINE_INDEX = 0xFFFF;
+
 	private final Type type;
 
 	private final RTCSessionDescription description;
@@ -104,6 +110,11 @@ public static SignalMessage of(RTCSessionDescription description) {
 	 * @param candidate a local ICE candidate.
 	 *
 	 * @return a message that carries the candidate.
+	 *
+	 * @throws IllegalArgumentException if the candidate's {@code
+	 *                                  sdpMLineIndex} is not from 0 to
+	 *                                  65535, which {@link #fromJson} would
+	 *                                  not read back.
 	 */
 	public static SignalMessage of(RTCIceCandidate candidate) {
 		requireNonNull(candidate, "candidate");
@@ -113,6 +124,8 @@ public static SignalMessage of(RTCIceCandidate candidate) {
 			return END_OF_CANDIDATES;
 		}
 
+		checkMLineIndex(candidate.sdpMLineIndex);
+
 		return new SignalMessage(Type.CANDIDATE, null, candidate);
 	}
 
@@ -130,10 +143,12 @@ public static SignalMessage endOfCandidates() {
 	 *
 	 * @return the message.
 	 *
-	 * @throws IllegalArgumentException if the text is no JSON or no signaling
-	 *                                  message, is longer than a MiB, or
-	 *                                  nests arrays and objects deeper than
-	 *                                  64 levels.
+	 * @throws IllegalArgumentException if the text is no strict JSON or no
+	 *                                  signaling message, is longer than a
+	 *                                  MiB, nests arrays and objects deeper
+	 *                                  than 64 levels, or has an {@code
+	 *                                  sdpMLineIndex} that is not an integer
+	 *                                  from 0 to 65535.
 	 */
 	public static SignalMessage fromJson(String json) {
 		requireNonNull(json, "json");
@@ -274,7 +289,7 @@ private static SignalMessage fromObject(Map<?, ?> object) {
 			}
 
 			return new SignalMessage(Type.CANDIDATE, null, new RTCIceCandidate(
-					(String) mid, index == null ? 0 : ((Number) index).intValue(), sdp));
+					(String) mid, index == null ? 0 : mLineIndex((Number) index), sdp));
 		}
 
 		Object type = object.get("type");
@@ -296,4 +311,32 @@ private static SignalMessage fromObject(Map<?, ?> object) {
 				throw new IllegalArgumentException("Unsupported description type \"" + type + "\"");
 		}
 	}
+
+	/**
+	 * @param number the {@code sdpMLineIndex} of a JSON message.
+	 *
+	 * @return the index, if the number is an integer from 0 to 65535.
+	 */
+	private static int mLineIndex(Number number) {
+		double index = number.doubleValue();
+
+		// Checked as a double, not after a cast, which would truncate 1.9 to 1
+		// and clamp 1e300 to Integer.MAX_VALUE.
+		if (index != Math.rint(index)) {
+			throw new IllegalArgumentException("\"sdpMLineIndex\" is not an integer: " + number);
+		}
+		if (index < 0 || index > MAX_M_LINE_INDEX) {
+			throw new IllegalArgumentException("\"sdpMLineIndex\" is not from 0 to " + MAX_M_LINE_INDEX
+					+ ": " + (Math.abs(index) < 1e15 ? Long.toString((long) index) : number.toString()));
+		}
+
+		return (int) index;
+	}
+
+	private static void checkMLineIndex(int index) {
+		if (index < 0 || index > MAX_M_LINE_INDEX) {
+			throw new IllegalArgumentException("\"sdpMLineIndex\" is not from 0 to " + MAX_M_LINE_INDEX
+					+ ": " + index);
+		}
+	}
 }