You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/03/29 15:08:39 UTC
[trafficserver] branch 9.2.x updated: [ink_base64] Fix buffer size computation (#8736)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new f8cde27 [ink_base64] Fix buffer size computation (#8736)
f8cde27 is described below
commit f8cde27bfcfa3e38073b623f6d47169580fdb1fe
Author: Mo Chen <un...@gmail.com>
AuthorDate: Mon Mar 28 17:22:23 2022 -0500
[ink_base64] Fix buffer size computation (#8736)
These buffer size computation macros are meant to be used for callers of
ats_base64_encode/decode. They were not taking into account a null
terminator, which is always written by those functions. This causes a
buffer overflow when encode or decode are called on buffers of certain
sizes, e.g. decode on an input buffer of size 4.
This change makes these buffer size calculations match the functions.
It also updates unit tests for the access_control plugin, which uses
these functions.
Since this is a macro change, plugins which use encode/decode will need to
be recompiled.
(cherry picked from commit 3c1006eb1984304a78cea140889290720584e9a0)
---
include/tscore/ink_base64.h | 15 +++++++++++++--
.../access_control/unit_tests/test_utils.cc | 18 +++++++++---------
plugins/experimental/access_control/utils.cc | 4 ++--
plugins/experimental/metalink/metalink.cc | 2 +-
src/tscore/ink_base64.cc | 6 ++----
5 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/include/tscore/ink_base64.h b/include/tscore/ink_base64.h
index a3c4d51..1473a31 100644
--- a/include/tscore/ink_base64.h
+++ b/include/tscore/ink_base64.h
@@ -42,5 +42,16 @@ bool ats_base64_encode(const unsigned char *inBuffer, size_t inBufferSize, char
bool ats_base64_decode(const char *inBuffer, size_t inBufferSize, unsigned char *outBuffer, size_t outBufSize, size_t *length);
// Little helper functions to calculate minimum required output buffer for encoding/decoding.
-#define ATS_BASE64_ENCODE_DSTLEN(_length) ((_length * 8) / 6 + 4)
-#define ATS_BASE64_DECODE_DSTLEN(_length) (((_length + 3) / 4) * 3)
+// These sizes include one byte for null termination, because ats_base64_encode and ats_base64_decode will always write a null
+// terminator.
+inline constexpr size_t
+ats_base64_encode_dstlen(size_t length)
+{
+ return ((length + 2) / 3) * 4 + 1;
+}
+
+inline constexpr size_t
+ats_base64_decode_dstlen(size_t length)
+{
+ return ((length + 3) / 4) * 3 + 1;
+}
diff --git a/plugins/experimental/access_control/unit_tests/test_utils.cc b/plugins/experimental/access_control/unit_tests/test_utils.cc
index 43b1f5c..5040964 100644
--- a/plugins/experimental/access_control/unit_tests/test_utils.cc
+++ b/plugins/experimental/access_control/unit_tests/test_utils.cc
@@ -36,7 +36,7 @@ TEST_CASE("Base64: estimate buffer size needed to encode a message", "[Base64][a
/* Test with a zero decoded message length */
encodedLen = cryptoBase64EncodedSize(0);
- CHECK(4 == encodedLen);
+ CHECK(1 == encodedLen);
/* Test with a random non-zero decoded message length */
encodedLen = cryptoBase64EncodedSize(64);
@@ -44,11 +44,11 @@ TEST_CASE("Base64: estimate buffer size needed to encode a message", "[Base64][a
/* Test the space for padding. Size of encoding that would result in 2 x "=" padding */
encodedLen = cryptoBase64EncodedSize(strlen("176a1620e31b14782ba2b66de3edc5b3cb19630475b2ce2ee292d5fd0fe41c3abc"));
- CHECK(92 == encodedLen);
+ CHECK(89 == encodedLen);
/* Test the space for padding. Size of encoding that would result in 1 x "=" padding */
encodedLen = cryptoBase64EncodedSize(strlen("176a1620e31b14782ba2b66de3edc5b3cb19630475b2ce2ee292d5fd0fe41c3ab"));
- CHECK(90 == encodedLen);
+ CHECK(89 == encodedLen);
/* Test the space for padding. Size of encoding that would result in no padding */
encodedLen = cryptoBase64EncodedSize(strlen("176a1620e31b14782ba2b66de3edc5b3cb19630475b2ce2ee292d5fd0fe41c3a"));
@@ -63,27 +63,27 @@ TEST_CASE("Base64: estimate buffer size needed to decode a message", "[Base64][a
/* Padding with 2 x '=' */
encoded = "MTc2YTE2MjBlMzFiMTQ3ODJiYTJiNjZkZTNlZGM1YjNjYjE5NjMwNDc1YjJjZTJlZTI5MmQ1ZmQwZmU0MWMzYQ==";
encodedLen = cryptoBase64DecodeSize(encoded, strlen(encoded));
- CHECK(66 == encodedLen);
+ CHECK(67 == encodedLen);
/* Padding with 1 x '=' */
encoded = "MTc2YTE2MjBlMzFiMTQ3ODJiYTJiNjZkZTNlZGM1YjNjYjE5NjMwNDc1YjJjZTJlZTI5MmQ1ZmQwZmU0MWMzYWI=";
encodedLen = cryptoBase64DecodeSize(encoded, strlen(encoded));
- CHECK(66 == encodedLen);
+ CHECK(67 == encodedLen);
/* Padding with 0 x "=" */
encoded = "MTc2YTE2MjBlMzFiMTQ3ODJiYTJiNjZkZTNlZGM1YjNjYjE5NjMwNDc1YjJjZTJlZTI5MmQ1ZmQwZmU0MWMzYWJj";
encodedLen = cryptoBase64DecodeSize(encoded, strlen(encoded));
- CHECK(66 == encodedLen);
+ CHECK(67 == encodedLen);
/* Test empty encoded message calculation */
encoded = "";
encodedLen = cryptoBase64DecodeSize(encoded, strlen(encoded));
- CHECK(0 == encodedLen);
+ CHECK(1 == encodedLen);
/* Test empty encoded message calculation */
encoded = nullptr;
encodedLen = cryptoBase64DecodeSize(encoded, 0);
- CHECK(0 == encodedLen);
+ CHECK(1 == encodedLen);
}
TEST_CASE("Base64: quick encode / decode", "[Base64][access_control][utility]")
@@ -103,7 +103,7 @@ TEST_CASE("Base64: quick encode / decode", "[Base64][access_control][utility]")
encodedMessageLen));
size_t decodedMessageEstimatedLen = cryptoBase64DecodeSize(encodedMessage, encodedMessageLen);
- CHECK(66 == decodedMessageEstimatedLen);
+ CHECK(67 == decodedMessageEstimatedLen);
char decodedMessage[encodedMessageEstimatedLen];
size_t decodedMessageLen = cryptoBase64Decode(encodedMessage, encodedMessageLen, decodedMessage, encodedMessageLen);
diff --git a/plugins/experimental/access_control/utils.cc b/plugins/experimental/access_control/utils.cc
index 53911e4..211716d 100644
--- a/plugins/experimental/access_control/utils.cc
+++ b/plugins/experimental/access_control/utils.cc
@@ -296,7 +296,7 @@ cryptoMessageDigestEqual(const char *md1, size_t md1Len, const char *md2, size_t
size_t
cryptoBase64EncodedSize(size_t decodedSize)
{
- return ATS_BASE64_ENCODE_DSTLEN(decodedSize);
+ return ats_base64_encode_dstlen(decodedSize);
}
/**
@@ -308,7 +308,7 @@ cryptoBase64EncodedSize(size_t decodedSize)
size_t
cryptoBase64DecodeSize(const char *encoded, size_t encodedLen)
{
- return ATS_BASE64_DECODE_DSTLEN(encodedLen);
+ return ats_base64_decode_dstlen(encodedLen);
}
/**
diff --git a/plugins/experimental/metalink/metalink.cc b/plugins/experimental/metalink/metalink.cc
index 9f7d201..b8eae9a 100644
--- a/plugins/experimental/metalink/metalink.cc
+++ b/plugins/experimental/metalink/metalink.cc
@@ -693,7 +693,7 @@ location_handler(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */)
const char *value;
int length;
- char digest[33]; /* ATS_BASE64_DECODE_DSTLEN() */
+ char digest[33]; /* ats_base64_decode_dstlen() */
SendData *data = static_cast<SendData *>(TSContDataGet(contp));
TSContDestroy(contp);
diff --git a/src/tscore/ink_base64.cc b/src/tscore/ink_base64.cc
index a1da352..bc68758 100644
--- a/src/tscore/ink_base64.cc
+++ b/src/tscore/ink_base64.cc
@@ -28,8 +28,6 @@
* authentication scheme does not require them. This implementation is
* intended for web-related use, and line breaks are not implemented.
*
- * These routines return char*'s to malloc-ed strings. The caller is
- * responsible for freeing the strings.
*/
#include "tscore/ink_platform.h"
#include "tscore/ink_base64.h"
@@ -44,7 +42,7 @@ ats_base64_encode(const unsigned char *inBuffer, size_t inBufferSize, char *outB
char *obuf = outBuffer;
char in_tail[4];
- if (outBufSize < ATS_BASE64_ENCODE_DSTLEN(inBufferSize)) {
+ if (outBufSize < ats_base64_encode_dstlen(inBufferSize)) {
return false;
}
@@ -130,7 +128,7 @@ ats_base64_decode(const char *inBuffer, size_t inBufferSize, unsigned char *outB
int inputBytesDecoded = 0;
// Make sure there is sufficient space in the output buffer
- if (outBufSize < ATS_BASE64_DECODE_DSTLEN(inBufferSize)) {
+ if (outBufSize < ats_base64_decode_dstlen(inBufferSize)) {
return false;
}