From: Brian Neradt Date: Sat, 21 May 2022 17:28:31 +0000 (+0100) Subject: Detect and handle chunk header size truncation X-Git-Tag: archive/raspbian/8.1.1+ds-1.1+rpi1+deb11u1^2~4 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=846754d14a53aab27d72377fc3583ee78b493032;p=trafficserver.git Detect and handle chunk header size truncation Origin: upstream Applied-Upstream: https://github.com/apache/trafficserver/commit/2addc8ca71449ceac0d5b80172460ee09c938f5e Reviewed-by: Jean Baptiste Favre Last-Update: 2022-05-21 This detects if a chunk header size is too large and, if so, closes the connection. Last-Update: 2022-05-21 Gbp-Pq: Name 0019-CVE_2021_37149.patch --- diff --git a/include/tscore/ink_memory.h b/include/tscore/ink_memory.h index cf850c0b..d3045226 100644 --- a/include/tscore/ink_memory.h +++ b/include/tscore/ink_memory.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -204,6 +205,24 @@ ink_zero(T &t) memset(static_cast(&t), 0, sizeof(t)); } +/** Verify that we can safely shift value num_places places left. + * + * This checks that the shift will not cause the variable to overflow and that + * the value will not become negative. + * + * @param[in] value The value against which to check whether the shift is safe. + * + * @param[in] num_places The number of places to check that shifting left is safe. + * + */ +template +inline constexpr bool +can_safely_shift_left(T value, int num_places) +{ + constexpr auto max_value = std::numeric_limits::max(); + return value >= 0 && value <= (max_value >> num_places); +} + /** Scoped resources. An instance of this class is used to hold a contingent resource. When this object goes out of scope diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc index 48aebb8d..a5ab2910 100644 --- a/proxy/http/HttpTunnel.cc +++ b/proxy/http/HttpTunnel.cc @@ -36,6 +36,7 @@ #include "HttpSM.h" #include "HttpDebugNames.h" #include "tscore/ParseRules.h" +#include "tscore/ink_memory.h" static const int min_block_transfer_bytes = 256; static const char *const CHUNK_HEADER_FMT = "%" PRIx64 "\r\n"; @@ -153,8 +154,16 @@ ChunkedHandler::read_size() if (state == CHUNK_READ_SIZE) { // The http spec says the chunked size is always in hex if (ParseRules::is_hex(*tmp)) { + // Make sure we will not overflow running_sum with our shift. + if (!can_safely_shift_left(running_sum, 4)) { + // We have no more space in our variable for the shift. + state = CHUNK_READ_ERROR; + done = true; + break; + } num_digits++; - running_sum *= 16; + // Shift over one hex value. + running_sum <<= 4; if (ParseRules::is_digit(*tmp)) { running_sum += *tmp - '0'; diff --git a/src/tscore/Makefile.am b/src/tscore/Makefile.am index a82aec24..f81d6ef1 100644 --- a/src/tscore/Makefile.am +++ b/src/tscore/Makefile.am @@ -258,6 +258,7 @@ test_tscore_SOURCES = \ unit_tests/test_BufferWriter.cc \ unit_tests/test_BufferWriterFormat.cc \ unit_tests/test_ink_inet.cc \ + unit_tests/test_ink_memory.cc \ unit_tests/test_IntrusivePtr.cc \ unit_tests/test_IpMap.cc \ unit_tests/test_layout.cc \ diff --git a/src/tscore/unit_tests/test_ink_memory.cc b/src/tscore/unit_tests/test_ink_memory.cc new file mode 100644 index 00000000..fa6725b8 --- /dev/null +++ b/src/tscore/unit_tests/test_ink_memory.cc @@ -0,0 +1,141 @@ +/** @file + + ink_memory unit tests. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF 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 + + http://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. +*/ + +#include +#include +#include "tscore/ink_memory.h" + +constexpr void +test_can_safely_shift_int8_t() +{ + constexpr int8_t a = 0; + static_assert(can_safely_shift_left(a, 0) == true, "shifting 0 is safe"); + static_assert(can_safely_shift_left(a, 4) == true, "shifting 0 is safe"); + static_assert(can_safely_shift_left(a, 8) == true, "shifting 0 is safe"); + + constexpr int8_t b = 1; + static_assert(can_safely_shift_left(b, 0) == true, "shifting int8_t 1 0 places is safe"); + static_assert(can_safely_shift_left(b, 1) == true, "shifting int8_t 1 1 places is safe"); + static_assert(can_safely_shift_left(b, 6) == true, "shifting int8_t 1 6 places is safe"); + static_assert(can_safely_shift_left(b, 7) == false, "shifting int8_t 1 7 places becomes negative"); + static_assert(can_safely_shift_left(b, 8) == false, "shifting int8_t 1 8 places overflows"); + + constexpr int8_t c = 0xff; + static_assert(can_safely_shift_left(c, 0) == false, "int8_t 0xff is already negative"); + static_assert(can_safely_shift_left(c, 1) == false, "shifting int8_t 0xff 1 place overflows"); +} + +constexpr void +test_can_safely_shift_uint8_t() +{ + constexpr uint8_t a = 0; + static_assert(can_safely_shift_left(a, 0) == true, "shifting 0 is safe"); + static_assert(can_safely_shift_left(a, 4) == true, "shifting 0 is safe"); + static_assert(can_safely_shift_left(a, 8) == true, "shifting 0 is safe"); + + constexpr uint8_t b = 1; + static_assert(can_safely_shift_left(b, 0) == true, "shifting uint8_t 1 0 places is safe"); + static_assert(can_safely_shift_left(b, 1) == true, "shifting uint8_t 1 1 places is safe"); + static_assert(can_safely_shift_left(b, 6) == true, "shifting uint8_t 1 6 places is safe"); + static_assert(can_safely_shift_left(b, 7) == true, "shifting uint8_t 1 7 is safe"); + static_assert(can_safely_shift_left(b, 8) == false, "shifting uint8_t 1 8 places overflows"); + + constexpr uint8_t c = 0xff; + static_assert(can_safely_shift_left(c, 0) == true, "shifting int8_t 0xff 0 places is safe"); + static_assert(can_safely_shift_left(c, 1) == false, "shifting int8_t 0xff 1 place overflows"); +} + +constexpr void +test_can_safely_shift_int32_t() +{ + constexpr int32_t a = 0; + static_assert(can_safely_shift_left(a, 4) == true, "shifting 0 is safe"); + + constexpr int32_t b = 1; + static_assert(can_safely_shift_left(b, 4) == true, "shifting 1 is safe"); + + constexpr int32_t c = 0x00ff'ffff; + static_assert(can_safely_shift_left(c, 4) == true, "shifting 0x00ff'ffff is safe"); + + constexpr int32_t d = 0x07ff'ffff; + static_assert(can_safely_shift_left(d, 4) == true, "shifting 0x07ff'ffff is safe"); + + constexpr int32_t e = -1; + static_assert(can_safely_shift_left(e, 4) == false, "shifting -1 will result in truncation"); + + constexpr int32_t f = 0x0800'0000; + static_assert(can_safely_shift_left(f, 4) == false, "shifting 0x0801'0000 will become negative"); + + constexpr int32_t g = 0x0fff'ffff; + static_assert(can_safely_shift_left(g, 4) == false, "shifting 0x0fff'ffff will become negative"); + + constexpr int32_t h = 0x1000'0000; + static_assert(can_safely_shift_left(h, 4) == false, "shifting 0x1000'0000 will overflow"); + + constexpr int32_t i = 0xf000'0000; + static_assert(can_safely_shift_left(i, 4) == false, "shifting 0xf000'0000 will overflow"); + + constexpr int32_t j = 0xf800'0000; + static_assert(can_safely_shift_left(j, 4) == false, "shifting 0xf800'0000 will become negative"); +} + +constexpr void +test_can_safely_shift_uint32_t() +{ + constexpr uint32_t a = 0; + static_assert(can_safely_shift_left(a, 4) == true, "shifting 0 is safe"); + + constexpr uint32_t b = 1; + static_assert(can_safely_shift_left(b, 4) == true, "shifting 1 is safe"); + + constexpr uint32_t c = 0x00ff'ffff; + static_assert(can_safely_shift_left(c, 4) == true, "shifting 0x00ff'ffff is safe"); + + constexpr uint32_t d = 0x07ff'ffff; + static_assert(can_safely_shift_left(d, 4) == true, "shifting 0x07ff'ffff is safe"); + + constexpr uint32_t e = 0x0800'0000; + static_assert(can_safely_shift_left(e, 4) == true, "shifting unisgned 0x0800'0000 is safe"); + + constexpr uint32_t f = 0x0fff'ffff; + static_assert(can_safely_shift_left(f, 4) == true, "shifting unsigned 0x0fff'ffff is safe"); + + constexpr uint32_t g = 0x1000'0000; + static_assert(can_safely_shift_left(g, 4) == false, "shifting 0x1000'0000 will overflow"); + + constexpr uint32_t h = 0xf000'0000; + static_assert(can_safely_shift_left(h, 4) == false, "shifting 0xf000'0000 will overflow"); + + constexpr uint32_t i = 0xf800'0000; + static_assert(can_safely_shift_left(i, 4) == false, "shifting 0xf800'0000 will become negative"); +} + +TEST_CASE("can_safely_shift", "[libts][ink_inet][memory]") +{ + // can_safely_shift_left is a constexpr function, therefore all these checks are + // done at compile time and REQUIRES calls are not necessary. + test_can_safely_shift_int8_t(); + test_can_safely_shift_uint8_t(); + test_can_safely_shift_int32_t(); + test_can_safely_shift_uint32_t(); +}