Integer overflow when parsing prefixed integers from qpack (#301)

* add testcase

* apply patch from issue

* update ci to also test 32bit

* fix MAX_POWER value and make sure 62 bit integers can still be processed

* fix existing tests with are no longer in the allowed range

* CI should install build tools

* test examples not as rust integration test. Instead run shell script in ci.

---------

Co-authored-by: Peter Michael Green <[email protected]>

Author: Ruben2424

.github/workflows/CI.yml

@@ -26,6 +26,8 @@ jobs:
       - test
       - doc
       - fuzz
+      - examples
+      - compliance
     steps:
       - run: exit 0
 
@@ -105,28 +107,43 @@ jobs:
           args: -p h3-quinn
 
   test:
-    name: Test ${{ matrix.toolchain }} ${{ matrix.os }}
+    name: Test ${{ matrix.toolchain }} ${{ matrix.os }} ${{ matrix.target }}
     needs: [style]
     strategy:
       matrix:
         os: [ubuntu-latest]
         toolchain: [stable, beta]
         features: [i-implement-a-third-party-backend-and-opt-into-breaking-changes, tracing, 'tracing,i-implement-a-third-party-backend-and-opt-into-breaking-changes']
+        target: [x86_64-unknown-linux-gnu]
+        include:
+          # Add a 32-bit target test configuration
+          - os: ubuntu-latest
+            toolchain: stable
+            features: i-implement-a-third-party-backend-and-opt-into-breaking-changes
+            target: i686-unknown-linux-gnu
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@v3
+      # Add this step for 32-bit build support
+      - name: Install 32-bit development libraries
+        if: matrix.target == 'i686-unknown-linux-gnu'
+        run: |
+          sudo dpkg --add-architecture i386
+          sudo apt-get update
+          sudo apt-get install -y gcc-multilib libc6-dev-i386
       - name: Install Rust ${{ matrix.toolchain }}
         uses: actions-rs/toolchain@v1
         with:
           profile: minimal
           toolchain: ${{ matrix.toolchain }}
+          target: ${{ matrix.target }}
           override: true
       - uses: Swatinem/rust-cache@v2
       - name: cargo test
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --features ${{ matrix.features }}
+          args: --features ${{ matrix.features }} --target ${{ matrix.target }}
       - name: h3Spec
         run: ./ci/h3spec.sh
         if: matrix.toolchain == 'stable'
@@ -186,3 +203,19 @@ jobs:
         uses: ./.github/actions/compliance
         with:
           report-script: ${{ github.workspace }}/ci/compliance/report.sh
+
+  examples:
+    name: Run Examples
+    needs: [test]
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install Rust stable
+        uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: stable
+          override: true
+      - uses: Swatinem/rust-cache@v2
+      - name: Run server and client examples test
+        run: ./ci/example_test.sh

ci/example_test.sh

@@ -0,0 +1,30 @@
+#!/bin/bash
+set -eo pipefail
+
+# Change to the repository root directory
+cd "$(dirname "$0")/.."
+
+# Start the server in the background
+echo "Starting server..."
+cargo run --example server -- --listen=[::]:4433 --cert=examples/server.cert --key=examples/server.key &
+SERVER_PID=$!
+
+# Wait for the server to start
+sleep 2
+
+# Function to clean up server process on exit
+cleanup() {
+  echo "Cleaning up server process..."
+  kill $SERVER_PID 2>/dev/null || true
+}
+
+# Set up cleanup on script exit
+trap cleanup EXIT
+
+# Run the client
+echo "Running client..."
+cargo run --example client -- https://localhost:4433 --ca=examples/ca.cert
+
+# If we got here, the test succeeded
+echo "Server and client connected successfully!"
+exit 0

h3/src/qpack/prefix_int.rs

@@ -77,11 +77,7 @@ pub fn encode<B: BufMut>(size: u8, flags: u8, value: u64, buf: &mut B) {
     buf.write(remaining as u8);
 }
 
-#[cfg(target_pointer_width = "64")]
-const MAX_POWER: usize = 10 * 7;
-
-#[cfg(target_pointer_width = "32")]
-const MAX_POWER: usize = 5 * 7;
+const MAX_POWER: usize = 9 * 7;
 
 impl From<coding::UnexpectedEnd> for Error {
     fn from(_: coding::UnexpectedEnd) -> Self {
@@ -91,8 +87,11 @@ impl From<coding::UnexpectedEnd> for Error {
 
 #[cfg(test)]
 mod test {
+    use assert_matches::assert_matches;
     use std::io::Cursor;
 
+    use crate::qpack::prefix_int::Error;
+
     fn check_codec(size: u8, flags: u8, value: u64, data: &[u8]) {
         let mut buf = Vec::new();
         super::encode(size, flags, value, &mut buf);
@@ -110,8 +109,8 @@ mod test {
         check_codec(
             5,
             0b010,
-            u64::max_value(),
-            &[95, 224, 255, 255, 255, 255, 255, 255, 255, 255, 1],
+            0x80_00_00_00_00_00_00_1E,
+            &[95, 255, 255, 255, 255, 255, 255, 255, 255, 127],
         );
     }
 
@@ -122,8 +121,8 @@ mod test {
         check_codec(
             8,
             0,
-            u64::max_value(),
-            &[255, 128, 254, 255, 255, 255, 255, 255, 255, 255, 1],
+            0x80_00_00_00_00_00_00_FE,
+            &[255, 255, 255, 255, 255, 255, 255, 255, 255, 127],
         );
     }
 
@@ -154,4 +153,22 @@ mod test {
     fn number_never_ends_with_0x80() {
         check_codec(4, 0b0001, 143, &[31, 128, 1]);
     }
+    #[test]
+    fn overflow2() {
+        let buf = vec![95, 225, 255, 255, 255, 255, 255, 255, 255, 255, 1];
+        let mut read = Cursor::new(&buf);
+        let x = super::decode(5, &mut read);
+        assert_matches!(x, Err(Error::Overflow));
+    }
+
+    #[test]
+    fn allow_62_bit() {
+        // This is the maximum value that can be encoded in with a flag size of 7 bits
+        // The value is requires more than 62 bits so the spec is fulfilled
+        let buf = vec![3, 255, 255, 255, 255, 255, 255, 255, 255, 127];
+        let mut read = Cursor::new(&buf);
+        let (flag, value) = super::decode(1, &mut read).expect("Value is allowed to be parsed");
+        assert_eq!(flag, 1);
+        assert_eq!(value, 9223372036854775808);
+    }
 }