From 6abb6e5110e36b7a6f7e8283607f4b326f102c29 Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Mon, 29 Aug 2022 21:12:27 +0000 Subject: [PATCH] [io/http] Validate method name passed to HttpClient.open/openUrl. There should be no control characters or delimiters in method name provided to open/openUrl methods. Fixes https://github.com/dart-lang/sdk/issues/45744 TEST=http_open_method_validate_test Change-Id: I0db98f2376c980a054420fb447d4f7ef9321f1a9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256429 Reviewed-by: Siva Annamalai Reviewed-by: Brian Quinlan Commit-Queue: Alexander Aprelev --- ...mock_http_headers.dart.weak.outline.expect | 8 ++--- sdk/lib/_http/http.dart | 6 +++- sdk/lib/_http/http_impl.dart | 30 +++++++++++++++++-- .../standalone/io/http_cookie_date_test.dart | 6 +++- tests/standalone/io/http_headers_test.dart | 6 +++- .../io/http_open_method_validate_test.dart | 28 +++++++++++++++++ tests/standalone/io/http_parser_test.dart | 6 +++- .../web_socket_protocol_processor_test.dart | 6 +++- .../io/http_open_method_validate_test.dart | 30 +++++++++++++++++++ 9 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 tests/standalone/io/http_open_method_validate_test.dart create mode 100644 tests/standalone_2/io/http_open_method_validate_test.dart diff --git a/pkg/front_end/testcases/nnbd_mixed/mock_http_headers.dart.weak.outline.expect b/pkg/front_end/testcases/nnbd_mixed/mock_http_headers.dart.weak.outline.expect index 0524cac834da..0e357f545572 100644 --- a/pkg/front_end/testcases/nnbd_mixed/mock_http_headers.dart.weak.outline.expect +++ b/pkg/front_end/testcases/nnbd_mixed/mock_http_headers.dart.weak.outline.expect @@ -170,8 +170,8 @@ Evaluated: MapLiteral @ org-dartlang-testcase:///mock_http_headers.dart:13:7 -> Evaluated: SymbolLiteral @ org-dartlang-testcase:///mock_http_headers.dart:13:7 -> SymbolConstant(#noFolding) Evaluated: ListLiteral @ org-dartlang-testcase:///mock_http_headers.dart:13:7 -> ListConstant(const []) Evaluated: MapLiteral @ org-dartlang-testcase:///mock_http_headers.dart:13:7 -> MapConstant(const {}) -Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:564:8 -> SymbolConstant(#clear) -Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:564:8 -> ListConstant(const []) -Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:564:8 -> ListConstant(const []) -Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:564:8 -> MapConstant(const {}) +Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:568:8 -> SymbolConstant(#clear) +Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:568:8 -> ListConstant(const []) +Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:568:8 -> ListConstant(const []) +Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/_http/http.dart:568:8 -> MapConstant(const {}) Extra constant evaluation: evaluated: 268, effectively constant: 91 diff --git a/sdk/lib/_http/http.dart b/sdk/lib/_http/http.dart index feaa042357f6..275380e5de8a 100644 --- a/sdk/lib/_http/http.dart +++ b/sdk/lib/_http/http.dart @@ -5,7 +5,11 @@ library dart._http; import 'dart:_internal' - show Since, valueOfNonNullableParamWithDefault, HttpStatus; + show + checkNotNullable, + Since, + valueOfNonNullableParamWithDefault, + HttpStatus; import 'dart:async'; import 'dart:collection' show diff --git a/sdk/lib/_http/http_impl.dart b/sdk/lib/_http/http_impl.dart index e1c0d5cbaf03..5e3c48f4d9b2 100644 --- a/sdk/lib/_http/http_impl.dart +++ b/sdk/lib/_http/http_impl.dart @@ -2678,6 +2678,30 @@ class _HttpClient implements HttpClient { } } + bool _isValidToken(String token) { + checkNotNullable(token, "token"); + // from https://www.rfc-editor.org/rfc/rfc2616#page-15 + // + // CTL = + // separators = "(" | ")" | "<" | ">" | "@" + // | "," | ";" | ":" | "\" | <"> + // | "/" | "[" | "]" | "?" | "=" + // | "{" | "}" | SP | HT + // token = 1* + const _validChars = r" " + r" ! #$%&' *+ -. 0123456789 " + r" ABCDEFGHIJKLMNOPQRSTUVWXYZ ^_" + r"`abcdefghijklmnopqrstuvwxyz | ~ "; + for (int codeUnit in token.codeUnits) { + if (codeUnit >= _validChars.length || + _validChars.codeUnitAt(codeUnit) == 0x20) { + return false; + } + } + return true; + } + Future<_HttpClientRequest> _openUrl(String method, Uri uri) { if (_closing) { throw StateError("Client is closed"); @@ -2686,9 +2710,11 @@ class _HttpClient implements HttpClient { // Ignore any fragments on the request URI. uri = uri.removeFragment(); - if (method == null) { - throw ArgumentError(method); + // from https://www.rfc-editor.org/rfc/rfc2616#page-35 + if (!_isValidToken(method)) { + throw ArgumentError.value(method, "method"); } + if (method != "CONNECT") { if (uri.host.isEmpty) { throw ArgumentError("No host specified in URI $uri"); diff --git a/tests/standalone/io/http_cookie_date_test.dart b/tests/standalone/io/http_cookie_date_test.dart index dc1787d69c06..fa79bf3c071b 100644 --- a/tests/standalone/io/http_cookie_date_test.dart +++ b/tests/standalone/io/http_cookie_date_test.dart @@ -16,7 +16,11 @@ import "dart:typed_data"; import "package:expect/expect.dart"; import "../../../sdk/lib/internal/internal.dart" - show Since, valueOfNonNullableParamWithDefault, HttpStatus; + show + checkNotNullable, + Since, + valueOfNonNullableParamWithDefault, + HttpStatus; part "../../../sdk/lib/_http/crypto.dart"; part "../../../sdk/lib/_http/embedder_config.dart"; diff --git a/tests/standalone/io/http_headers_test.dart b/tests/standalone/io/http_headers_test.dart index 99884f646109..69fefa3d4daf 100644 --- a/tests/standalone/io/http_headers_test.dart +++ b/tests/standalone/io/http_headers_test.dart @@ -16,7 +16,11 @@ import "dart:typed_data"; import "package:expect/expect.dart"; import "../../../sdk/lib/internal/internal.dart" - show Since, valueOfNonNullableParamWithDefault, HttpStatus; + show + checkNotNullable, + Since, + valueOfNonNullableParamWithDefault, + HttpStatus; part "../../../sdk/lib/_http/crypto.dart"; part "../../../sdk/lib/_http/embedder_config.dart"; diff --git a/tests/standalone/io/http_open_method_validate_test.dart b/tests/standalone/io/http_open_method_validate_test.dart new file mode 100644 index 000000000000..484a4abbf650 --- /dev/null +++ b/tests/standalone/io/http_open_method_validate_test.dart @@ -0,0 +1,28 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +// +// Verify that HttpClient open, openUrl method argument is validated. + +import "dart:io"; +import "package:expect/expect.dart"; + +void testInvalidArgumentException(String method) { + Expect.throws(() => HttpClient()..open(method, "127.0.0.1", 8080, "/"), + (e) => e is ArgumentError); + Expect.throws( + () => HttpClient()..openUrl(method, Uri.parse("http://127.0.0.1/")), + (e) => e is ArgumentError); +} + +main() { + const String separators = "\t\n\r()<>@,;:\\/[]?={}"; + for (int i = 0; i < separators.length; i++) { + String separator = separators.substring(i, i + 1); + testInvalidArgumentException(separator); + testInvalidArgumentException(separator + "CONNECT"); + testInvalidArgumentException("CONN" + separator + "ECT"); + testInvalidArgumentException("CONN" + separator + separator + "ECT"); + testInvalidArgumentException("CONNECT" + separator); + } +} diff --git a/tests/standalone/io/http_parser_test.dart b/tests/standalone/io/http_parser_test.dart index 300ed639be90..e55e75c69892 100644 --- a/tests/standalone/io/http_parser_test.dart +++ b/tests/standalone/io/http_parser_test.dart @@ -16,7 +16,11 @@ import "dart:typed_data"; import "package:expect/expect.dart"; import "../../../sdk/lib/internal/internal.dart" - show Since, valueOfNonNullableParamWithDefault, HttpStatus; + show + checkNotNullable, + Since, + valueOfNonNullableParamWithDefault, + HttpStatus; part "../../../sdk/lib/_http/crypto.dart"; part "../../../sdk/lib/_http/embedder_config.dart"; diff --git a/tests/standalone/io/web_socket_protocol_processor_test.dart b/tests/standalone/io/web_socket_protocol_processor_test.dart index 344a0b93c5b2..a3a57a5f482e 100644 --- a/tests/standalone/io/web_socket_protocol_processor_test.dart +++ b/tests/standalone/io/web_socket_protocol_processor_test.dart @@ -17,7 +17,11 @@ import "package:async_helper/async_helper.dart"; import "package:expect/expect.dart"; import "../../../sdk/lib/internal/internal.dart" - show Since, valueOfNonNullableParamWithDefault, HttpStatus; + show + checkNotNullable, + Since, + valueOfNonNullableParamWithDefault, + HttpStatus; part "../../../sdk/lib/_http/crypto.dart"; part "../../../sdk/lib/_http/embedder_config.dart"; diff --git a/tests/standalone_2/io/http_open_method_validate_test.dart b/tests/standalone_2/io/http_open_method_validate_test.dart new file mode 100644 index 000000000000..668f570a300b --- /dev/null +++ b/tests/standalone_2/io/http_open_method_validate_test.dart @@ -0,0 +1,30 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +// +// Verify that HttpClient open, openUrl method argument is validated. + +// @dart = 2.9 + +import "dart:io"; +import "package:expect/expect.dart"; + +void testInvalidArgumentException(String method) { + Expect.throws(() => HttpClient()..open(method, "127.0.0.1", 8080, "/"), + (e) => e is ArgumentError); + Expect.throws( + () => HttpClient()..openUrl(method, Uri.parse("http://127.0.0.1/")), + (e) => e is ArgumentError); +} + +main() { + const String separators = "\t\n\r()<>@,;:\\/[]?={}"; + for (int i = 0; i < separators.length; i++) { + String separator = separators.substring(i, i + 1); + testInvalidArgumentException(separator); + testInvalidArgumentException(separator + "CONNECT"); + testInvalidArgumentException("CONN" + separator + "ECT"); + testInvalidArgumentException("CONN" + separator + separator + "ECT"); + testInvalidArgumentException("CONNECT" + separator); + } +}