This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch 9.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit d5806bd715b83cb31477493dc5b1997437a95b54
Author: Mo Chen <uncorr...@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 a3c4d510a..1473a312a 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 43b1f5c49..5040964aa 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 53911e463..211716da5 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 9f7d201a4..b8eae9a8c 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 a1da352f3..bc68758bc 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;
}
