java-topology/defects/undertow/patch/undertow-0001-websocket-subprotocol-negotiation.md

144 lines
5.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# UNDF: UNDF-2026-000000561
## undertow-0001 — CWE-407: DefaultContainerConfigurator.getNegotiatedSubprotocol() List.contains() O(R×S) per WebSocket handshake
**File:** `websockets-jsr/src/main/java/io/undertow/websockets/jsr/DefaultContainerConfigurator.java`
**Method:** `getNegotiatedSubprotocol(List<String> supported, List<String> requested)`
**Severity:** MEDIUM
**CWE:** CWE-407 (Inefficient Algorithmic Complexity)
### Defect
`getNegotiatedSubprotocol()` iterates over the client-provided `requested` subprotocol
list and calls `supported.contains(proto)` on each iteration. `supported` is a
`List<String>` (from `ServerEndpointConfig.getSubprotocols()` which returns
`List<String>` per the Jakarta WebSocket spec). `List.contains()` is O(S) — it
performs a linear scan via `String.equals()`. The outer loop runs |R| times,
giving total complexity O(|R| × |S|).
```java
// DEFECTIVE — websockets-jsr/.../DefaultContainerConfigurator.java:50-57
@Override
public String getNegotiatedSubprotocol(final List<String> supported, final List<String> requested) {
for(String proto : requested) {
if(supported.contains(proto)) { // O(S) scan per iteration → O(R×S) total
return proto;
}
}
return "";
}
```
A hostile client can send |R| = 100+ subprotocol values in the
`Sec-WebSocket-Protocol` header. With S server-configured protocols this
becomes O(R × S) string comparisons per WebSocket upgrade request, executed
on the I/O thread.
The companion method `getNegotiatedExtensions()` has the same pattern:
```java
// DEFECTIVE — nested O(|requested| × |installed|) loop
for (Extension req : requested) {
for (Extension extension : installed) {
if (extension.getName().equals(req.getName())) { ...
```
### Fix
Convert `supported` to a `HashSet<String>` once before the loop, replacing
O(S) per-call with O(1). For extensions, build a `Map<String, Extension>` on
`installed` keyed by name.
```java
// FIXED
@Override
public String getNegotiatedSubprotocol(final List<String> supported, final List<String> requested) {
// Build O(1)-lookup set from server-side list once, not O(S) per iteration.
Set<String> supportedSet = new HashSet<>(supported);
for (String proto : requested) {
if (supportedSet.contains(proto)) {
return proto;
}
}
return "";
}
@Override
public List<Extension> getNegotiatedExtensions(final List<Extension> installed, final List<Extension> requested) {
// Build O(1)-lookup map from installed extensions keyed by name.
Map<String, Extension> installedMap = new HashMap<>(installed.size() * 2);
for (Extension ext : installed) {
installedMap.put(ext.getName(), ext);
}
final List<Extension> ret = new ArrayList<>();
for (Extension req : requested) {
if (installedMap.containsKey(req.getName())) {
ret.add(req);
}
}
return ret;
}
```
Imports to add: `java.util.HashMap`, `java.util.HashSet`, `java.util.Map`, `java.util.Set`
### Complexity
| | Before | After |
|---|---|---|
| getNegotiatedSubprotocol() | O(\|R\| × \|S\|) | O(\|R\| + \|S\|) |
| getNegotiatedExtensions() | O(\|req\| × \|inst\|) | O(\|req\| + \|inst\|) |
### Impact
Called on every WebSocket upgrade handshake on the I/O thread. A client
sending 100 requested subprotocols against a server with 50 configured
subprotocols produces 5000 string comparisons. Fix reduces to 150.
### Patch
```diff
--- a/websockets-jsr/src/main/java/io/undertow/websockets/jsr/DefaultContainerConfigurator.java
+++ b/websockets-jsr/src/main/java/io/undertow/websockets/jsr/DefaultContainerConfigurator.java
@@ -22,6 +22,9 @@ import io.undertow.servlet.api.InstanceHandle;
import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
import jakarta.websocket.Extension;
@@ -50,17 +53,21 @@ public class DefaultContainerConfigurator extends ServerEndpointConfig.Configura
@Override
public String getNegotiatedSubprotocol(final List<String> supported, final List<String> requested) {
- for(String proto : requested) {
- if(supported.contains(proto)) {
+ Set<String> supportedSet = new HashSet<>(supported);
+ for (String proto : requested) {
+ if (supportedSet.contains(proto)) {
return proto;
}
}
return "";
}
@Override
public List<Extension> getNegotiatedExtensions(final List<Extension> installed, final List<Extension> requested) {
+ Map<String, Extension> installedMap = new HashMap<>(installed.size() * 2);
+ for (Extension ext : installed) {
+ installedMap.put(ext.getName(), ext);
+ }
final List<Extension> ret = new ArrayList<>();
for (Extension req : requested) {
- for (Extension extension : installed) {
- if (extension.getName().equals(req.getName())) {
- ret.add(req);
- break;
- }
+ if (installedMap.containsKey(req.getName())) {
+ ret.add(req);
}
}
return ret;
}
```