From 86e099c1342c90d308037d1a72515697e85f1106 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 27 Feb 2019 10:13:18 +0000 Subject: [PATCH 1/5] gbase64: Convert a precondition from g_error() to g_return_val_if_fail() The caller needs to check this themselves in any case, so we might as well at least follow convention in defining the precondition. Signed-off-by: Philip Withnall --- glib/gbase64.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/glib/gbase64.c b/glib/gbase64.c index ae9d5fef55..d3416e310b 100644 --- a/glib/gbase64.c +++ b/glib/gbase64.c @@ -268,9 +268,7 @@ g_base64_encode (const guchar *data, /* We can use a smaller limit here, since we know the saved state is 0, +1 is needed for trailing \0, also check for unlikely integer overflow */ - if (len >= ((G_MAXSIZE - 1) / 4 - 1) * 3) - g_error("%s: input too large for Base64 encoding (%"G_GSIZE_FORMAT" chars)", - G_STRLOC, len); + g_return_val_if_fail (len < ((G_MAXSIZE - 1) / 4 - 1) * 3, NULL); out = g_malloc ((len / 3 + 1) * 4 + 1); -- GitLab From ff76f6920e36f5befff270ba318bc612d1585839 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 27 Feb 2019 10:26:47 +0000 Subject: [PATCH 2/5] gbase64: Allow g_base64_encode (NULL, 0) and g_base64_decode ("", *) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Relax a precondition in g_base64_encode_step() to allow this. It’s valid to base64 encode an empty string, as per RFC 4648. Similarly for g_base64_decode(), although calling it with a NULL string has never been allowed. Instead, clarify the case of calling it with an empty string. This includes a unit test. Signed-off-by: Philip Withnall Fixes: #1698 --- glib/gbase64.c | 8 ++++---- glib/tests/base64.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/glib/gbase64.c b/glib/gbase64.c index d3416e310b..baddae0984 100644 --- a/glib/gbase64.c +++ b/glib/gbase64.c @@ -100,7 +100,7 @@ g_base64_encode_step (const guchar *in, char *outptr; const guchar *inptr; - g_return_val_if_fail (in != NULL, 0); + g_return_val_if_fail (in != NULL || len == 0, 0); g_return_val_if_fail (out != NULL, 0); g_return_val_if_fail (state != NULL, 0); g_return_val_if_fail (save != NULL, 0); @@ -244,7 +244,7 @@ g_base64_encode_close (gboolean break_lines, /** * g_base64_encode: - * @data: (array length=len) (element-type guint8): the binary data to encode + * @data: (array length=len) (element-type guint8) (nullable): the binary data to encode * @len: the length of @data * * Encode a sequence of binary data into its Base-64 stringified @@ -334,7 +334,7 @@ g_base64_decode_step (const gchar *in, unsigned int v; int i; - g_return_val_if_fail (in != NULL, 0); + g_return_val_if_fail (in != NULL || len == 0, 0); g_return_val_if_fail (out != NULL, 0); g_return_val_if_fail (state != NULL, 0); g_return_val_if_fail (save != NULL, 0); @@ -390,7 +390,7 @@ g_base64_decode_step (const gchar *in, /** * g_base64_decode: - * @text: zero-terminated string with base64 text to decode + * @text: (not nullable): zero-terminated string with base64 text to decode * @out_len: (out): The length of the decoded data is written here * * Decode a sequence of Base-64 encoded text into binary data. Note diff --git a/glib/tests/base64.c b/glib/tests/base64.c index 86875a29b1..6091d1eede 100644 --- a/glib/tests/base64.c +++ b/glib/tests/base64.c @@ -406,6 +406,42 @@ test_base64_decode_smallblock (gconstpointer blocksize_p) } } +/* Test that calling g_base64_encode (NULL, 0) returns correct output. This is + * as per the first test vector in RFC 4648 §10. + * https://tools.ietf.org/html/rfc4648#section-10 */ +static void +test_base64_encode_empty (void) +{ + gchar *encoded = NULL; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1698"); + + encoded = g_base64_encode (NULL, 0); + g_assert_cmpstr (encoded, ==, ""); + g_free (encoded); + + encoded = g_base64_encode ((const guchar *) "", 0); + g_assert_cmpstr (encoded, ==, ""); + g_free (encoded); +} + +/* Test that calling g_base64_decode ("", *) returns correct output. This is + * as per the first test vector in RFC 4648 §10. Note that calling + * g_base64_decode (NULL, *) is not allowed. + * https://tools.ietf.org/html/rfc4648#section-10 */ +static void +test_base64_decode_empty (void) +{ + guchar *decoded = NULL; + gsize decoded_len; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1698"); + + decoded = g_base64_decode ("", &decoded_len); + g_assert_cmpstr ((gchar *) decoded, ==, ""); + g_assert_cmpuint (decoded_len, ==, 0); + g_free (decoded); +} int main (int argc, char *argv[]) @@ -455,5 +491,8 @@ main (int argc, char *argv[]) g_test_add_data_func ("/base64/incremental/smallblock/4", GINT_TO_POINTER(4), test_base64_decode_smallblock); + g_test_add_func ("/base64/encode/empty", test_base64_encode_empty); + g_test_add_func ("/base64/decode/empty", test_base64_decode_empty); + return g_test_run (); } -- GitLab From f9dfddf8eb4952fc5e0a77fffeff70c7cea37b68 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 27 Feb 2019 10:27:43 +0000 Subject: [PATCH 3/5] gbase64: Fix an impossible condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit len is unsigned, so it’s not possible for it to be less than zero. Signed-off-by: Philip Withnall --- glib/gbase64.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/glib/gbase64.c b/glib/gbase64.c index baddae0984..ef6a398324 100644 --- a/glib/gbase64.c +++ b/glib/gbase64.c @@ -105,7 +105,7 @@ g_base64_encode_step (const guchar *in, g_return_val_if_fail (state != NULL, 0); g_return_val_if_fail (save != NULL, 0); - if (len <= 0) + if (len == 0) return 0; inptr = in; @@ -339,7 +339,7 @@ g_base64_decode_step (const gchar *in, g_return_val_if_fail (state != NULL, 0); g_return_val_if_fail (save != NULL, 0); - if (len <= 0) + if (len == 0) return 0; inend = (const guchar *)in+len; -- GitLab From 387e76287934097c924f70c8047280932f4be61e Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 27 Feb 2019 10:35:24 +0000 Subject: [PATCH 4/5] =?UTF-8?q?tests:=20Add=20base64=20tests=20from=20?= =?UTF-8?q?=C2=A7(Test=20Vectors)=20of=20RFC=204648?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While I’m here, we might as well check that we output what the RFC says we should output. https://tools.ietf.org/html/rfc4648#section-10 (We do.) Signed-off-by: Philip Withnall --- glib/tests/base64.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/glib/tests/base64.c b/glib/tests/base64.c index 6091d1eede..bb9d4db721 100644 --- a/glib/tests/base64.c +++ b/glib/tests/base64.c @@ -443,6 +443,49 @@ test_base64_decode_empty (void) g_free (decoded); } +/* Check all the RFC 4648 test vectors for base 64 encoding from §10. + * https://tools.ietf.org/html/rfc4648#section-10 */ +static void +test_base64_encode_decode_rfc4648 (void) +{ + const struct + { + const gchar *decoded; /* technically this should be a byte array, but all the test vectors are ASCII strings */ + const gchar *encoded; + } + vectors[] = + { + { "", "" }, + { "f", "Zg==" }, + { "fo", "Zm8=" }, + { "foo", "Zm9v" }, + { "foob", "Zm9vYg==" }, + { "fooba", "Zm9vYmE=" }, + { "foobar", "Zm9vYmFy" }, + }; + gsize i; + + for (i = 0; i < G_N_ELEMENTS (vectors); i++) + { + gchar *encoded = NULL; + guchar *decoded = NULL; + gsize expected_decoded_len = strlen (vectors[i].decoded); + gsize decoded_len; + + g_test_message ("Vector %" G_GSIZE_FORMAT ": %s", i, vectors[i].decoded); + + encoded = g_base64_encode ((const guchar *) vectors[i].decoded, expected_decoded_len); + g_assert_cmpstr (encoded, ==, vectors[i].encoded); + + decoded = g_base64_decode (encoded, &decoded_len); + g_assert_cmpstr ((gchar *) decoded, ==, vectors[i].decoded); + g_assert_cmpuint (decoded_len, ==, expected_decoded_len); + + g_free (encoded); + g_free (decoded); + } +} + int main (int argc, char *argv[]) { @@ -494,5 +537,7 @@ main (int argc, char *argv[]) g_test_add_func ("/base64/encode/empty", test_base64_encode_empty); g_test_add_func ("/base64/decode/empty", test_base64_decode_empty); + g_test_add_func ("/base64/encode-decode/rfc4648", test_base64_encode_decode_rfc4648); + return g_test_run (); } -- GitLab From 2484d1c9500ccff8f33c1955193d1a1c45696eb0 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 27 Feb 2019 11:20:14 +0000 Subject: [PATCH 5/5] gbase64: Remove an unnecessary condition At that point in the code, len can only be 0, 1 or 2. The code below is a no-op if (len == 0), so the condition is pointless. Remove it, and we should be able to achieve full branch coverage of gbase64.c. This should introduce no functional changes. Signed-off-by: Philip Withnall --- glib/gbase64.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/glib/gbase64.c b/glib/gbase64.c index ef6a398324..dd7ed20ac8 100644 --- a/glib/gbase64.c +++ b/glib/gbase64.c @@ -160,7 +160,8 @@ g_base64_encode_step (const guchar *in, *state = already; } - if (len>0) + g_assert (len == 0 || len == 1 || len == 2); + { char *saveout; -- GitLab