From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56129) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6kB4-0000Hr-74 for qemu-devel@nongnu.org; Wed, 09 Dec 2015 14:13:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a6kB0-0006cN-4W for qemu-devel@nongnu.org; Wed, 09 Dec 2015 14:13:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38314) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a6kAz-0006cI-Qv for qemu-devel@nongnu.org; Wed, 09 Dec 2015 14:13:22 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id E80DA935D8 for ; Wed, 9 Dec 2015 19:13:20 +0000 (UTC) References: <1449670091-5891-1-git-send-email-berrange@redhat.com> <1449670091-5891-2-git-send-email-berrange@redhat.com> From: John Snow Message-ID: <56687D4F.9060201@redhat.com> Date: Wed, 9 Dec 2015 14:13:19 -0500 MIME-Version: 1.0 In-Reply-To: <1449670091-5891-2-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 1/5] util: add base64 decoding function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Markus Armbruster On 12/09/2015 09:08 AM, Daniel P. Berrange wrote: > The standard glib provided g_base64_decode doesn't provide any > kind of sensible error checking on its input. Add a QEMU custom > wrapper qbase64_decode which can be used with untrustworthy > input that can contain invalid base64 characters, embedded > NUL characters, or not be NUL terminated at all. > > Signed-off-by: Daniel P. Berrange > --- > include/qemu/base64.h | 58 +++++++++++++++++++++++++++ > tests/.gitignore | 1 + > tests/Makefile | 3 ++ > tests/test-base64.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++ > util/Makefile.objs | 1 + > util/base64.c | 60 +++++++++++++++++++++++++++ > 6 files changed, 232 insertions(+) > create mode 100644 include/qemu/base64.h > create mode 100644 tests/test-base64.c > create mode 100644 util/base64.c > > diff --git a/include/qemu/base64.h b/include/qemu/base64.h > new file mode 100644 > index 0000000..793708d > --- /dev/null > +++ b/include/qemu/base64.h > @@ -0,0 +1,58 @@ > +/* > + * QEMU base64 helpers > + * > + * Copyright (c) 2015 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see . > + * > + */ > + > +#ifndef QEMU_BASE64_H__ > +#define QEMU_BASE64_H__ > + > +#include "qemu-common.h" > + > + > +/** > + * qbase64_decode: > + * @input: the (possibly) base64 encoded text > + * @in_len: length of @input or -1 if NUL terminated > + * @out_len: filled with length of decoded data > + * @errp: pointer to a NULL-initialized error object > + * > + * Attempt to decode the (possibly) base64 encoded > + * text provided in @input. If the @input text may > + * contain embedded NUL characters, or may not be > + * NUL terminated, then @in_len must be set to the > + * known size of the @input buffer. > + * > + * Note that embedded NULs, or lack of a NUL terminator > + * are considered invalid base64 data and errors > + * will be reported to this effect. > + * > + * If decoding is successful, the decoded data will > + * be returned and @out_len set to indicate the > + * number of bytes in the decoded data. The caller > + * must use g_free() to free the returned data when > + * it is no longer required. > + * > + * Returns: the decoded data or NULL > + */ > +uint8_t *qbase64_decode(const char *input, > + size_t in_len, > + size_t *out_len, > + Error **errp); > + > + > +#endif /* QEMU_BUFFER_H__ */ > diff --git a/tests/.gitignore b/tests/.gitignore > index 1e55722..00a7fed 100644 > --- a/tests/.gitignore > +++ b/tests/.gitignore > @@ -8,6 +8,7 @@ check-qom-interface > check-qom-proplist > rcutorture > test-aio > +test-base64 > test-bitops > test-blockjob-txn > test-coroutine > diff --git a/tests/Makefile b/tests/Makefile > index a1d03b4..7dc8437 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -84,6 +84,7 @@ check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF) > check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF) > check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF) > check-unit-y += tests/test-timed-average$(EXESUF) > +check-unit-y += tests/test-base64$(EXESUF) > > check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh > > @@ -416,6 +417,8 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ > $(test-qom-obj-y) > tests/test-timed-average$(EXESUF): tests/test-timed-average.o qemu-timer.o \ > $(test-util-obj-y) > +tests/test-base64$(EXESUF): tests/test-base64.o \ > + libqemuutil.a libqemustub.a > > tests/test-qapi-types.c tests/test-qapi-types.h :\ > $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > diff --git a/tests/test-base64.c b/tests/test-base64.c > new file mode 100644 > index 0000000..f40eb94 > --- /dev/null > +++ b/tests/test-base64.c > @@ -0,0 +1,109 @@ > +/* > + * QEMU base64 heper test > + * helper? > + * Copyright (c) 2015 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see . > + * > + */ > + > +#include > + > +#include "qemu/base64.h" > + > + > +static void test_base64_good(void) > +{ > + const char input[] = > + "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\n" > + "lzc2VkIHRoZSBzY29ycGlvbi4="; > + const char expect[] = "Because we focused on the snake, " > + "we missed the scorpion."; > + :) > + size_t len; > + uint8_t *actual = qbase64_decode(input, > + -1, > + &len, > + &error_abort); > + > + g_assert(actual != NULL); > + g_assert_cmpint(len, ==, strlen(expect)); > + g_assert_cmpstr((char *)actual, ==, expect); > + g_free(actual); > +} > + > + > +static void test_base64_bad(const char *input, > + size_t input_len) > +{ > + size_t len; > + Error *err = NULL; > + uint8_t *actual = qbase64_decode(input, > + input_len, > + &len, > + &err); > + > + g_assert(err != NULL); > + g_assert(actual == NULL); > + g_assert_cmpint(len, ==, 0); > + error_free(err); > +} > + > + > +static void test_base64_embedded_nul(void) > +{ > + /* We put a NUL character in the middle of the base64 > + * text which is invalid data, given the expected length */ > + const char input[] = > + "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\0" > + "lzc2VkIHRoZSBzY29ycGlvbi4="; > + > + test_base64_bad(input, G_N_ELEMENTS(input) - 1); > +} > + > + > +static void test_base64_not_nul_terminated(void) > +{ > + const char input[] = > + "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW\n" > + "lzc2VkIHRoZSBzY29ycGlvbi4="; > + > + /* Using '-2' to make us drop the trailing NUL, thus > + * creating an invalid base64 sequence for decoding */ > + test_base64_bad(input, G_N_ELEMENTS(input) - 2); > +} > + > + > +static void test_base64_invalid_chars(void) > +{ > + /* We put a single quote character in the middle > + * of the base64 text which is invalid data */ > + const char input[] = > + "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZSBzbmFrZSwgd2UgbW'" > + "lzc2VkIHRoZSBzY29ycGlvbi4="; > + > + test_base64_bad(input, strlen(input)); > +} > + > + > +int main(int argc, char **argv) > +{ > + g_test_init(&argc, &argv, NULL); > + g_test_add_func("/util/base64/good", test_base64_good); > + g_test_add_func("/util/base64/embedded-nul", test_base64_embedded_nul); > + g_test_add_func("/util/base64/not-nul-terminated", > + test_base64_not_nul_terminated); > + g_test_add_func("/util/base64/invalid-chars", test_base64_invalid_chars); > + return g_test_run(); > +} > diff --git a/util/Makefile.objs b/util/Makefile.objs > index 89dd80e..8620a80 100644 > --- a/util/Makefile.objs > +++ b/util/Makefile.objs > @@ -30,3 +30,4 @@ util-obj-y += qemu-coroutine-sleep.o > util-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o > util-obj-y += buffer.o > util-obj-y += timed-average.o > +util-obj-y += base64.o > diff --git a/util/base64.c b/util/base64.c > new file mode 100644 > index 0000000..f82caa7 > --- /dev/null > +++ b/util/base64.c > @@ -0,0 +1,60 @@ > +/* > + * QEMU base64 helpers > + * > + * Copyright (c) 2015 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see . > + * > + */ > + > +#include > + > +#include "qemu/base64.h" > + > +static const char *base64_valid_chars = > + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=\n"; > + > +uint8_t *qbase64_decode(const char *input, > + size_t in_len, > + size_t *out_len, > + Error **errp) > +{ > + *out_len = 0; > + > + if (in_len != -1) { > + /* Lack of NUL terminator is an error */ > + if (input[in_len] != '\0') { > + error_setg(errp, "Base64 data is not NUL terminated"); > + return NULL; > + } > + /* Check there's no NULs embedded since we expect > + * this to be valid base64 data */ > + if (memchr(input, '\0', in_len) != NULL) { > + error_setg(errp, "Base64 data contains embedded NUL characters"); > + return NULL; > + } > + > + /* Now we know its a valid nul terminated string > + * strspn is safe to use... */ > + } else { > + in_len = strlen(input); > + } > + > + if (strspn(input, base64_valid_chars) != in_len) { > + error_setg(errp, "Base64 data contains invalid characters"); > + return NULL; > + } > + > + return g_base64_decode(input, out_len); > +} >