Skip to content

Commit 24013f5

Browse files
authored
fix: enforce the request-smuggling (CL.TE) check at the proxy chokepoint (#598)
* fix: enforce CL.TE smuggling check at the proxy chokepoint, drop broken CL.0 body-read (#14) * fix: enforce HTTP/1.x request-smuggling checks at the proxy chokepoint The CL.0/CL.TE check lived inside StandardEndpointMapper, so custom or test mappers bypassed it; the CL.0 body-read also blocked the event loop without ever catching the smuggle, since the decoder splits CL:0 + trailing bytes into two pipelined requests. Run the check before map() and use a per- connection taint flag to reject the next request after a Content-Length: 0. * revert: drop the per-connection CL.0 taint A legitimate Content-Length: 0 request followed by another request on the same keep-alive connection is routine (CORS preflight, empty-body POSTs, HEAD/DELETE/OPTIONS), so tainting the connection caused collateral 400s on normal flows. The proxy layer cannot reliably distinguish a pipelined-burst smuggle from sequential keep-alive reuse without channel-buffer inspection, so keep only the header-only CL.TE check and accept that pure CL.0 smuggling is not addressed at this layer. * fix: smuggling check runs before filters can mutate CL/TE Move the rejectAsSmuggling decision above the filter loop in processRequest so a user-supplied request filter that strips or rewrites Content-Length or Transfer-Encoding cannot bypass the check. The smuggling boolean is captured on the raw inbound headers; filters and the mapper run as before. * fix: reject detected smuggling before the filter chain, not just the mapper A request flagged as CL.TE smuggling was still run through every request filter before being mapped to a 400. Short-circuit to MapResult.badRequest() so a detected attack skips both the filters and the mapper. Detection still runs on the raw inbound headers first (in the if-condition), so a filter cannot mask it.
1 parent bf8bdb7 commit 24013f5

2 files changed

Lines changed: 19 additions & 30 deletions

File tree

carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,15 @@ private static void addCustomResponseHeaders(final HttpHeaders responseHeaders,
142142
});
143143
}
144144

145+
private static boolean rejectAsSmuggling(ProxyRequest request) {
146+
final HttpHeaders headers = request.getRequestHeaders();
147+
if (headers.get(HttpHeaderNames.CONTENT_LENGTH) != null && headers.get(HttpHeaderNames.TRANSFER_ENCODING) != null) {
148+
LOGGER.warn("Potential CL.TE request smuggling attack: both Content-Length and Transfer-Encoding present. Request: {}", request.getUri());
149+
return true;
150+
}
151+
return false;
152+
}
153+
145154
private static void cleanRequestFromCacheValidators(ProxyRequest request) {
146155
HttpHeaders headers = request.getRequestHeaders();
147156
headers.remove(HttpHeaderNames.IF_MATCH);
@@ -209,10 +218,16 @@ public Publisher<Void> processRequest(ProxyRequest request) {
209218
request.setLastActivity(request.getStartTs());
210219
request.getRequestHeaders().set(HttpHeaderNames.SERVER, ServerHeaderRequestFilter.DEFAULT_SERVER);
211220

212-
parent.getFilters().forEach(filter -> filter.apply(request));
213-
214-
MapResult action = parent.getMapper().map(request);
215-
221+
// HTTP/1.x request-smuggling validation runs on the raw inbound headers, before any request filter
222+
// or the mapper, so a filter that mutates Content-Length / Transfer-Encoding cannot bypass it.
223+
// HTTP/2 framing is not vulnerable.
224+
final MapResult action;
225+
if (request.getHttpProtocol().majorVersion() < 2 && rejectAsSmuggling(request)) {
226+
action = MapResult.badRequest();
227+
} else {
228+
parent.getFilters().forEach(filter -> filter.apply(request));
229+
action = parent.getMapper().map(request);
230+
}
216231
request.setAction(action);
217232

218233
if (LOGGER.isTraceEnabled()) {

carapace-server/src/main/java/org/carapaceproxy/server/mapper/StandardEndpointMapper.java

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
import com.google.common.net.HostAndPort;
2727
import com.google.common.net.InetAddresses;
2828
import com.google.common.net.InternetDomainName;
29-
import io.netty.handler.codec.http.HttpHeaderNames;
30-
import io.netty.handler.codec.http.HttpHeaders;
3129
import io.netty.handler.codec.http.HttpResponseStatus;
3230
import java.util.ArrayList;
3331
import java.util.Collections;
@@ -149,30 +147,6 @@ public MapResult map(final ProxyRequest request) {
149147
return MapResult.badRequest();
150148
}
151149

152-
// CL.0 and CL.TE request smuggling validation
153-
// HTTP/2 requests are not vulnerable to these attacks, so we only check HTTP/1.x
154-
if (request.getHttpProtocol().majorVersion() < 2) {
155-
HttpHeaders headers = request.getRequestHeaders();
156-
String contentLength = headers.get(HttpHeaderNames.CONTENT_LENGTH);
157-
String transferEncoding = headers.get(HttpHeaderNames.TRANSFER_ENCODING);
158-
159-
// Check for CL.TE attack: both Content-Length and Transfer-Encoding headers present
160-
if (contentLength != null && transferEncoding != null) {
161-
LOG.warn("Potential CL.TE request smuggling attack detected: both Content-Length and Transfer-Encoding headers present. Request: {}", request.getUri());
162-
return MapResult.badRequest();
163-
}
164-
165-
// Check for CL.0 attack: Content-Length: 0 but with body
166-
if (contentLength != null && contentLength.trim().equals("0")) {
167-
// It's OK to consume the string in the flux, as it should be empty anyway
168-
final String body = request.getRequestData().asString().blockFirst();
169-
if (body != null && !body.isEmpty()) {
170-
LOG.warn("Request with Content-Length: 0 but with body detected: {}", request.getUri());
171-
return MapResult.badRequest();
172-
}
173-
}
174-
}
175-
176150
for (final RouteConfiguration route : routes) {
177151
if (!route.isEnabled()) {
178152
continue;

0 commit comments

Comments
 (0)