From 1c99ac2d8e785b157594750b2c939d4b2c441854 Mon Sep 17 00:00:00 2001
From: Cory Benfield <lukasa@apple.com>
Date: Mon, 15 Apr 2019 09:37:53 +0100
Subject: [PATCH] Resolve off-by-one errors in settings bounds.

Motivation:

When we're validating that settings are in their bounds, we should probably endeavour to
express those bounds correctly.

Modifications:

- Fixed the bounds checks on SETTINGS_INITIAL_WINDOW_SIZE and SETTINGS_MAX_FRAME_SIZE.
- Added regression tests.

Result:

Better correctness.
---
 .../ConnectionStateMachine.swift              |  4 ++--
 .../ConnectionStateMachineTests+XCTest.swift  |  2 ++
 .../ConnectionStateMachineTests.swift         | 20 +++++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift b/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
index 395b75d5..06612027 100644
--- a/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
+++ b/Sources/NIOHTTP2/ConnectionStateMachine/ConnectionStateMachine.swift
@@ -1304,11 +1304,11 @@ extension HTTP2ConnectionStateMachine {
                     return .connectionError(underlyingError: NIOHTTP2Errors.InvalidSetting(setting: setting), type: .protocolError)
                 }
             case .initialWindowSize:
-                guard setting._value < HTTP2FlowControlWindow.maxSize else {
+                guard setting._value <= HTTP2FlowControlWindow.maxSize else {
                     return .connectionError(underlyingError: NIOHTTP2Errors.InvalidSetting(setting: setting), type: .flowControlError)
                 }
             case .maxFrameSize:
-                guard setting._value >= (1 << 14) && setting._value < ((1 << 24) - 1) else {
+                guard setting._value >= (1 << 14) && setting._value <= ((1 << 24) - 1) else {
                     return .connectionError(underlyingError: NIOHTTP2Errors.InvalidSetting(setting: setting), type: .protocolError)
                 }
             default:
diff --git a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
index 154b88e0..d5776070 100644
--- a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
+++ b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests+XCTest.swift
@@ -122,6 +122,8 @@ extension ConnectionStateMachineTests {
                 ("testRejectHeadersWithTEHeaderNotTrailers", testRejectHeadersWithTEHeaderNotTrailers),
                 ("testAllowHeadersWithTEHeaderNotTrailersWhenValidationDisabled", testAllowHeadersWithTEHeaderNotTrailersWhenValidationDisabled),
                 ("testAllowHeadersWithTEHeaderSetToTrailers", testAllowHeadersWithTEHeaderSetToTrailers),
+                ("testSettingActualMaxFrameSize", testSettingActualMaxFrameSize),
+                ("testSettingActualInitialWindowSize", testSettingActualInitialWindowSize),
            ]
    }
 }
diff --git a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
index d81e60d4..0d464a4d 100644
--- a/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
+++ b/Tests/NIOHTTP2Tests/ConnectionStateMachineTests.swift
@@ -2498,6 +2498,26 @@ class ConnectionStateMachineTests: XCTestCase {
         assertSucceeds(self.client.sendHeaders(streamID: streamOne, headers: ConnectionStateMachineTests.requestHeaders.withExtraHeaders(invalidExtraHeaders), isEndStreamSet: true))
         assertSucceeds(self.server.receiveHeaders(streamID: streamOne, headers: ConnectionStateMachineTests.requestHeaders.withExtraHeaders(invalidExtraHeaders), isEndStreamSet: true))
     }
+
+    func testSettingActualMaxFrameSize() {
+        self.exchangePreamble()
+
+        // It must be possible to set SETTINGS_MAX_FRAME_SIZE to (2**24)-1.
+        let trickySettings: HTTP2Settings = [HTTP2Setting(parameter: .maxFrameSize, value: (1<<24) - 1)]
+        assertSucceeds(self.client.sendSettings(trickySettings))
+        assertSucceeds(self.server.receiveSettings(.settings(trickySettings), frameEncoder: &self.serverEncoder, frameDecoder: &self.serverDecoder))
+        assertSucceeds(self.client.receiveSettings(.ack, frameEncoder: &self.clientEncoder, frameDecoder: &self.clientDecoder))
+    }
+
+    func testSettingActualInitialWindowSize() {
+        self.exchangePreamble()
+
+        // It must be possible to set SETTINGS_INITIAL_WINDOW_SIZE to (2**31)-1.
+        let trickySettings: HTTP2Settings = [HTTP2Setting(parameter: .initialWindowSize, value: (1<<31) - 1)]
+        assertSucceeds(self.client.sendSettings(trickySettings))
+        assertSucceeds(self.server.receiveSettings(.settings(trickySettings), frameEncoder: &self.serverEncoder, frameDecoder: &self.serverDecoder))
+        assertSucceeds(self.client.receiveSettings(.ack, frameEncoder: &self.clientEncoder, frameDecoder: &self.clientDecoder))
+    }
 }