CVE-2024-40642: Insufficient Information (NVD-CWE-noinfo)
value: The netty incubator codec.bhttp is a java language binary http parser. In affected versions the `BinaryHttpParser` class does not properly validate input values thus giving attackers almost complete control over the HTTP requests constructed from the parsed output. Attackers can abuse several issues individually to perform various injection attacks including HTTP request smuggling, desync attacks, HTTP header injections, request queue poisoning, caching attacks and Server Side Request Forgery (SSRF). Attacker could also combine several issues to create well-formed messages for other text-based protocols which may result in attacks beyond the HTTP protocol. The BinaryHttpParser class implements the readRequestHead method which performs most of the relevant parsing of the received request. The data structure prefixes values with a variable length integer value. The parsing code below first gets the lengths of the values from the prefixed variable length integer. After it has all of the lengths and calculates all of the indices, the parser casts the applicable slices of the ByteBuf to String. Finally, it passes these values into a new `DefaultBinaryHttpRequest` object where no further parsing or validation occurs. Method is partially validated while other values are not validated at all. Software that relies on netty to apply input validation for binary HTTP data may be vulnerable to various injection and protocol based attacks. This issue has been addressed in version 0.0.13.Final. Users are advised to upgrade. There are no known workarounds for this vulnerability.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-18T23:15Z
Repository: https://github.com/netty/netty-incubator-codec-ohttp
Language: Java
Files changed: codec-bhttp/src/main/java/io/netty/incubator/codec/bhttp/BinaryHttpParser.java, codec-bhttp/src/test/java/io/netty/incubator/codec/bhttp/BinaryHttpParserTest.java
Commit date: 2024-07-18 10:04:46 -1000
Commit message: Merge commit from fork

Motivation:

We need to validate the various parts of the http request head before constructing it to guard against things like injections.

Modifications:

- Validate method, scheme, authority and path
- Unit tests

Result:

No injection possible anymore
Diff: diff --git a/codec-bhttp/src/main/java/io/netty/incubator/codec/bhttp/BinaryHttpParser.java b/codec-bhttp/src/main/java/io/netty/incubator/codec/bhttp/BinaryHttpParser.java
index c592d1b..ce864d0 100644
--- a/codec-bhttp/src/main/java/io/netty/incubator/codec/bhttp/BinaryHttpParser.java
+++ b/codec-bhttp/src/main/java/io/netty/incubator/codec/bhttp/BinaryHttpParser.java
@@ -71,16 +71,55 @@ public final class BinaryHttpParser {
         State(boolean knownLength) {
             this.knownLength = knownLength;
         }
     }
 
+    private static final boolean[] ALLOWED_TOKEN;
+    private static final boolean[] ALLOWED_SCHEME;
+
+    static {
+        ALLOWED_TOKEN = new boolean[256];
+        for (byte b = Byte.MIN_VALUE; b < Byte.MAX_VALUE; b++) {
+            ALLOWED_TOKEN[128 + b] = !Character.isWhitespace(b);
+        }
+
+        // See https://www.rfc-editor.org/rfc/rfc3986.html
+        //    scheme        = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+        ALLOWED_SCHEME = new boolean[256];
+        for (byte b = Byte.MIN_VALUE; b < Byte.MAX_VALUE; b++) {
+            ALLOWED_SCHEME[128 + b] = Character.isAlphabetic(b) || Character.isDigit(b) ||
+                    b == (byte) '+' || b == (byte) '-' || b == (byte) '.';
+        }
+    }
+
+    // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.2
+    private static final ByteProcessor TOKEN_VALIDATOR = b -> {
+        // Is whitespace will match whitespaces and delimiters.
+        if (ALLOWED_TOKEN[b + 128]) {
+            return true;
+        }
+        throw new IllegalArgumentException(
+                "Invalid char in token received: '" + b + "' (0x" + Integer.toHexString(b) + ")");
+    };
+
+    // See https://www.rfc-editor.org/rfc/rfc3986.html
+    private static final ByteProcessor SCHEME_VALIDATOR = b -> {
+        // Is whitespace will match whitespaces and delimiters.
+        if (ALLOWED_SCHEME[b + 128]) {
+            return true;
+        }
+        throw new IllegalArgumentException(
+                "Invalid char in scheme received : '" + b + "' (0x" + Integer.toHexString(b) + ")");
+    };
+
     private static final ByteProcessor PADDING_VALIDATOR = b -> {
         // Let's validate that only 0 is used for padding. While this is not strictly required
         // it can't harm to enforce it.
         // See https://www.rfc-editor.org/rfc/rfc9292.html#section-3.8
         if (b != 0) {
-            throw new CorruptedFrameException("Invalid byte used for padding: " + b);
+            throw new CorruptedFrameException(
+                    "Invalid byte used for padding: '" + b + "' (0x" + Integer.toHexString(b) + ")");
         }
         return true;
     };
 
     private State state = State.READ_FRAME_TYPE;
@@ -416,10 +455,18 @@ public final class BinaryHttpParser {
             return null;
         }
         // Add the bytes of the field section as well.
         sumBytes += fieldSectionReadableBytes - fieldSectionSlice.readableBytes();
 
+        // Let's validate method, scheme, authority and path.
+        in.forEachByte(methodIdx, (int) methodLength, TOKEN_VALIDATOR);
+        in.forEachByte(schemeIdx, (int) schemeLength, SCHEME_VALIDATOR);
+
+        // We only do very limited validation for these to ensure there can nothing be injected.
+        in.forEachByte(authorityIdx, (int) authorityLength, TOKEN_VALIDATOR);
+        in.forEachByte(pathIdx, (int) pathLength, TOKEN_VALIDATOR);
+
         String method = in.toString(methodIdx, (int) methodLength, StandardCharsets.US_ASCII);
         String scheme = in.toString(schemeIdx, (int) schemeLength, StandardCharsets.US_ASCII);
         String authority = in.toString(authorityIdx, (int) authorityLength, StandardCharsets.US_ASCII);
         String path = in.toString(pathIdx, (int) pathLength, StandardCharsets.US_ASCII);
 
diff --git a/codec-bhttp/src/test/java/io/netty/incubator/codec/bhttp/BinaryHttpParserTest.java b/codec-bhttp/src/test/java/io/netty/incubator/codec/bhttp/BinaryHttpParserTest.java
new file mode 100644
index 0000000..da52038
--- /dev/null
+++ b/codec-bhttp/src/test/java/io/netty/incubator/codec/bhttp/BinaryHttpParserTest.java
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2024 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.incubator.codec.bhttp;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import io.netty.util.CharsetUtil;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+public class BinaryHttpParserTest {
+
+    @ParameterizedTest(name = "{index} => {0}, {1}, {2}")
+    @MethodSource("invalidChars")
+    void testInvalidMethodSuffix(Position p, Part part, String hexString, Character c) {
+        ByteBuf buffer = Unpooled.buffer();
+        VarIntCodecUtils.writeVariableLengthInteger(buffer, 0);
+
+        writeString(Part.METHOD, "GET", buffer, p, part, c);
+        writeString(Part.SCHEME, "HTTPS", buffer, p, part, c);
+        writeString(Part.AUTHORITY, "something", buffer, p, part, c);
+        writeString(Part.PATH, "/somepath", buffer, p, part, c);
+        VarIntCodecUtils.writeVariableLengthInteger(buffer, 0);
+        testInvalidHead(buffer);
+    }
+
+    private void writeString(Part currentPart, String str, ByteBuf out, Position p, Part part, Character c) {
+        if (currentPart == part) {
+            switch (p) {
+                case PREFIX:
+                    writeString(out, c + str);
+                    break;
+                case SUFFIX:
+                    writeString(out, str + c);
+                    break;
+                case MIDDLE:
+                    writeString(out, str.substring(0, 1) + c + str.substring(1));
+                    break;
+            }
+        } else {
+            writeString(out, str);
+        }
+    }
+    private static Stream<Arguments> invalidChars() {
+        List<Arguments> invalid = new ArrayList<>();
+        for (int i = Byte.MIN_VALUE; i < Byte.MAX_VALUE; i++) {
+            char c = (char) i;
+            if (Character.isWhitespace(c)) {
+                for (Position p: Position.values()) {
+                    for (Part part: Part.values()) {
+                        invalid.add(Arguments.of(p, part, "0x" + Integer.toHexString(c), c));
+                    }
+                }
+            }
+        }
+        return invalid.stream();
+    }
+
+    private static void writeString(ByteBuf out, String str) {
+        byte[] bytes = str.getBytes(CharsetUtil.US_ASCII);
+        VarIntCodecUtils.writeVariableLengthInteger(out, bytes.length);
+        out.writeBytes(bytes);
+    }
+
+    private static void testInvalidHead(ByteBuf input) {
+        BinaryHttpParser parser = new BinaryHttpParser(8192);
+        Assertions.assertThrows(IllegalArgumentException.class, () -> parser.parse(input, false));
+        input.release();
+    }
+
+    private enum Position {
+        PREFIX,
+        MIDDLE,
+        SUFFIX;
+    }
+
+    private enum Part {
+        METHOD,
+        SCHEME,
+        AUTHORITY,
+        PATH
+    }
+}

Vulnerable code:             throw new CorruptedFrameException("Invalid byte used for padding: " + b);
Fixed code:     private static final boolean[] ALLOWED_TOKEN;
    private static final boolean[] ALLOWED_SCHEME;

    static {
        ALLOWED_TOKEN = new boolean[256];
        for (byte b = Byte.MIN_VALUE; b < Byte.MAX_VALUE; b++) {
            ALLOWED_TOKEN[128 + b] = !Character.isWhitespace(b);
        }

        // See https://www.rfc-editor.org/rfc/rfc3986.html
        //    scheme        = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
        ALLOWED_SCHEME = new boolean[256];
        for (byte b = Byte.MIN_VALUE; b < Byte.MAX_VALUE; b++) {
            ALLOWED_SCHEME[128 + b] = Character.isAlphabetic(b) || Character.isDigit(b) ||
                    b == (byte) '+' || b == (byte) '-' || b == (byte) '.';
        }
    }

    // See https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.2
    private static final ByteProcessor TOKEN_VALIDATOR = b -> {
        // Is whitespace will match whitespaces and delimiters.
        if (ALLOWED_TOKEN[b + 128]) {
            return true;
        }
        throw new IllegalArgumentException(
                "Invalid char in token received: '" + b + "' (0x" + Integer.toHexString(b) + ")");
    };

    // See https://www.rfc-editor.org/rfc/rfc3986.html
    private static final ByteProcessor SCHEME_VALIDATOR = b -> {
        // Is whitespace will match whitespaces and delimiters.
        if (ALLOWED_SCHEME[b + 128]) {
            return true;
        }
        throw new IllegalArgumentException(
                "Invalid char in scheme received : '" + b + "' (0x" + Integer.toHexString(b) + ")");
    };

            throw new CorruptedFrameException(
                    "Invalid byte used for padding: '" + b + "' (0x" + Integer.toHexString(b) + ")");
        // Let's validate method, scheme, authority and path.
        in.forEachByte(methodIdx, (int) methodLength, TOKEN_VALIDATOR);
        in.forEachByte(schemeIdx, (int) schemeLength, SCHEME_VALIDATOR);

        // We only do very limited validation for these to ensure there can nothing be injected.
        in.forEachByte(authorityIdx, (int) authorityLength, TOKEN_VALIDATOR);
        in.forEachByte(pathIdx, (int) pathLength, TOKEN_VALIDATOR);

/*
 * Copyright 2024 The Netty Project
 *
 * The Netty Project licenses this file to you under the Apache License,
 * version 2.0 (the "License"); you may not use this file except in compliance
 * with the License. You may obtain a copy of the License at:
 *
 *   https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
 * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 * License for the specific language governing permissions and limitations
 * under the License.
 */
package io.netty.incubator.codec.bhttp;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.util.CharsetUtil;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;

public class BinaryHttpParserTest {

    @ParameterizedTest(name = "{index} => {0}, {1}, {2}")
    @MethodSource("invalidChars")
    void testInvalidMethodSuffix(Position p, Part part, String hexString, Character c) {
        ByteBuf buffer = Unpooled.buffer();
        VarIntCodecUtils.writeVariableLengthInteger(buffer, 0);

        writeString(Part.METHOD, "GET", buffer, p, part, c);
        writeString(Part.SCHEME, "HTTPS", buffer, p, part, c);
        writeString(Part.AUTHORITY, "something", buffer, p, part, c);
        writeString(Part.PATH, "/somepath", buffer, p, part, c);
        VarIntCodecUtils.writeVariableLengthInteger(buffer, 0);
        testInvalidHead(buffer);
    }

    private void writeString(Part currentPart, String str, ByteBuf out, Position p, Part part, Character c) {
        if (currentPart == part) {
            switch (p) {
                case PREFIX:
                    writeString(out, c + str);
                    break;
                case SUFFIX:
                    writeString(out, str + c);
                    break;
                case MIDDLE:
                    writeString(out, str.substring(0, 1) + c + str.substring(1));
                    break;
            }
        } else {
            writeString(out, str);
        }
    }
    private static Stream<Arguments> invalidChars() {
        List<Arguments> invalid = new ArrayList<>();
        for (int i = Byte.MIN_VALUE; i < Byte.MAX_VALUE; i++) {
            char c = (char) i;
            if (Character.isWhitespace(c)) {
                for (Position p: Position.values()) {
                    for (Part part: Part.values()) {
                        invalid.add(Arguments.of(p, part, "0x" + Integer.toHexString(c), c));
                    }
                }
            }
        }
        return invalid.stream();
    }

    private static void writeString(ByteBuf out, String str) {
        byte[] bytes = str.getBytes(CharsetUtil.US_ASCII);
        VarIntCodecUtils.writeVariableLengthInteger(out, bytes.length);
        out.writeBytes(bytes);
    }

    private static void testInvalidHead(ByteBuf input) {
        BinaryHttpParser parser = new BinaryHttpParser(8192);
        Assertions.assertThrows(IllegalArgumentException.class, () -> parser.parse(input, false));
        input.release();
    }

    private enum Position {
        PREFIX,
        MIDDLE,
        SUFFIX;
    }

    private enum Part {
        METHOD,
        SCHEME,
        AUTHORITY,
        PATH
    }
}