Re: [OpenWrt-Devel] [PATCH] [libubox][v3] b64: add base64 support
Hi, On Mon, Apr 13, 2015 at 2:34 AM, Luka Perkov wrote: > The base code has been taken from zstream project which was > written by Steven Barth. > > Signed-off-by: Luka Perkov > CC: Steven Barth > --- > => changes in v2: > > Use new API: > > size_t b64decode(void **out, const char *in, size_t len); > size_t b64encode(char **out, const void *in, size_t len); > > => changes in v3: > > Use new API: > > static inline int b64_decode_size(const void *in, size_t len); > static inline int b64_encode_size(size_t len); > > size_t b64decode(void *out, const void *in, size_t len); > size_t b64encode(void *out, const void *in, size_t len); > > In this set a few corner cases of invalid writes in b64decode were fixed. > Furthermore, b64decode() works fine when in == out. > > CMakeLists.txt | 2 +- > b64.c | 112 > + > b64.h | 60 +++ > 3 files changed, 173 insertions(+), 1 deletion(-) > create mode 100644 b64.c > create mode 100644 b64.h > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 58381da..77f4842 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -14,7 +14,7 @@ IF(JSONC_FOUND) >INCLUDE_DIRECTORIES(${JSONC_INCLUDE_DIRS}) > ENDIF() > > -SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c > ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c) > +SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c > ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c > b64.c) > > ADD_LIBRARY(ubox SHARED ${SOURCES}) > ADD_LIBRARY(ubox-static STATIC ${SOURCES}) > diff --git a/b64.c b/b64.c > new file mode 100644 > index 000..007fb91 > --- /dev/null > +++ b/b64.c > @@ -0,0 +1,112 @@ > +/* > + * Copyright (C) 2011 Steven Barth > + * Copyright (C) 2015 Luka Perkov > + * > + * Permission to use, copy, modify, and/or distribute this software for any > + * purpose with or without fee is hereby granted, provided that the above > + * copyright notice and this permission notice appear in all copies. > + * > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + */ > + > +#include > +#include > + > +#include "b64.h" > + > +static const uint8_t b64decode_tbl[] = { > + 0x3e, 0xff, 0xff, 0xff, 0x3f, 0x34, 0x35, 0x36, > + 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0xff, > + 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x01, > + 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, > + 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, > + 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, > + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x1a, 0x1b, > + 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23, > + 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, > + 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x32, 0x33 > +}; > + > +size_t b64decode(void *out, const void *in, size_t len) > +{ > + uint8_t *o = (uint8_t *) out; > + const uint8_t *data = (const uint8_t *) in; > + size_t lenout, i, j; > + > + lenout = b64_decode_size(in, len); > + if (!lenout) > + return 0; > + > + uint32_t cv = 0; > + for (i = 0; i < (len - 4); i += 4) { > + cv = 0; > + for (j = 0; j < 4; j++) { > + uint8_t c = data[i + j] - 43; > + if (c > 79 || (c = b64decode_tbl[c]) == 0xff) > + return 0; > + > + cv |= c; > + if (j != 3) > + cv <<= 6; > + } > + > + o[2] = (uint8_t)(cv & 0xff); > + o[1] = (uint8_t)((cv >> 8) & 0xff); > + o[0] = (uint8_t)((cv >> 16) & 0xff); > + o += 3; > + } > + > + if (data[len - 1] != '=') > + o[2] = (uint8_t)(cv & 0xff); > + > + if (data[len - 1] != '=') > + o[1] = (uint8_t)((cv >> 8) & 0xff); You have the same condition twice, either you intended to have them different and this is a bug or you can merge them. Regards Jonas ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] [libubox][v3] b64: add base64 support
On 2015-04-14 18:24, Luka Perkov wrote: > Hi, > > On Tue, Apr 14, 2015 at 06:00:32PM +0200, Felix Fietkau wrote: >> >> + if (data[len - 1] == '=') >> >> + ret--; >> >> + >> >> + if (data[len - 2] == '=') >> >> + ret--; >> > >> > the 2 if clauses look redundant and i guess you could solve it with a loop > > Ok. I'll fix it. > >> I'd prefer not passing in the input buffer here at all - a tiny >> overestimation of the decode size doesn't hurt. > > I don't think that is a good idea since we do not have NULL terminating > output buffer in b64decode(). If you really want a function without > input buffer we can make another one called: > > static inline int b64_decode_size_approx(size_t len); > > The overestimation is likely going to cause problems in cases like this: > > buf = malloc(b64_decode_size(data, data_len)); > if (!buf) > return -1 > > buf_len = b64decode(rbuf, data, data_len); > if (!buf_len) > return -1 > > write(fd, buf, buf_len); > > In this case end of buf could be foobared and because of that extra > bytes could be written. As long as buf_len only returns the actual number of output bytes, I don't see how overestimation is going to cause problems here. Also, I think it would be nice to have a 0-terminated output. The extra 0-byte should be counted only in b64_decode_size and not in the result of b64decode. I think that will make the API harder to misuse, as calling C-string functions on the result will be safe by default. - Felix ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] [libubox][v3] b64: add base64 support
Hi, On Tue, Apr 14, 2015 at 06:00:32PM +0200, Felix Fietkau wrote: > >> + if (data[len - 1] == '=') > >> + ret--; > >> + > >> + if (data[len - 2] == '=') > >> + ret--; > > > > the 2 if clauses look redundant and i guess you could solve it with a loop Ok. I'll fix it. > I'd prefer not passing in the input buffer here at all - a tiny > overestimation of the decode size doesn't hurt. I don't think that is a good idea since we do not have NULL terminating output buffer in b64decode(). If you really want a function without input buffer we can make another one called: static inline int b64_decode_size_approx(size_t len); The overestimation is likely going to cause problems in cases like this: buf = malloc(b64_decode_size(data, data_len)); if (!buf) return -1 buf_len = b64decode(rbuf, data, data_len); if (!buf_len) return -1 write(fd, buf, buf_len); In this case end of buf could be foobared and because of that extra bytes could be written. Luka ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] [libubox][v3] b64: add base64 support
On 2015-04-14 17:41, John Crispin wrote: > hi > > comments inline > > On 13/04/2015 02:34, Luka Perkov wrote: >> The base code has been taken from zstream project which was >> written by Steven Barth. >> >> Signed-off-by: Luka Perkov >> CC: Steven Barth >> --- >> --- /dev/null >> +++ b/b64.h >> @@ -0,0 +1,60 @@ >> +/* >> + * Copyright (C) 2011 Steven Barth >> + * Copyright (C) 2015 Luka Perkov >> + * >> + * Permission to use, copy, modify, and/or distribute this software for any >> + * purpose with or without fee is hereby granted, provided that the above >> + * copyright notice and this permission notice appear in all copies. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES >> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF >> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR >> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES >> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN >> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF >> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. >> + */ >> + >> +#ifndef __LIBUBOX_B64_H >> +#define __LIBUBOX_B64_H >> + >> +#include >> + >> +static inline int b64_decode_size(const void *in, size_t len) >> +{ >> +const uint8_t *data = (const uint8_t *) in; >> +int ret; >> + >> +if ((len == 0) || (len % 4)) >> +return 0; >> + >> +ret = (len / 4) * 3; >> + >> +if (data[len - 1] == '=') >> +ret--; >> + >> +if (data[len - 2] == '=') >> +ret--; > > the 2 if clauses look redundant and i guess you could solve it with a loop I'd prefer not passing in the input buffer here at all - a tiny overestimation of the decode size doesn't hurt. - Felix ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] [libubox][v3] b64: add base64 support
hi comments inline On 13/04/2015 02:34, Luka Perkov wrote: > The base code has been taken from zstream project which was > written by Steven Barth. > > Signed-off-by: Luka Perkov > CC: Steven Barth > --- > => changes in v2: > > Use new API: > > size_t b64decode(void **out, const char *in, size_t len); > size_t b64encode(char **out, const void *in, size_t len); > > => changes in v3: > > Use new API: > > static inline int b64_decode_size(const void *in, size_t len); > static inline int b64_encode_size(size_t len); > > size_t b64decode(void *out, const void *in, size_t len); > size_t b64encode(void *out, const void *in, size_t len); > > In this set a few corner cases of invalid writes in b64decode were fixed. > Furthermore, b64decode() works fine when in == out. > > CMakeLists.txt | 2 +- > b64.c | 112 > + > b64.h | 60 +++ > 3 files changed, 173 insertions(+), 1 deletion(-) > create mode 100644 b64.c > create mode 100644 b64.h > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 58381da..77f4842 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -14,7 +14,7 @@ IF(JSONC_FOUND) >INCLUDE_DIRECTORIES(${JSONC_INCLUDE_DIRS}) > ENDIF() > > -SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c > ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c) > +SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c > ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c > b64.c) > > ADD_LIBRARY(ubox SHARED ${SOURCES}) > ADD_LIBRARY(ubox-static STATIC ${SOURCES}) > diff --git a/b64.c b/b64.c > new file mode 100644 > index 000..007fb91 > --- /dev/null > +++ b/b64.c > @@ -0,0 +1,112 @@ > +/* > + * Copyright (C) 2011 Steven Barth > + * Copyright (C) 2015 Luka Perkov > + * > + * Permission to use, copy, modify, and/or distribute this software for any > + * purpose with or without fee is hereby granted, provided that the above > + * copyright notice and this permission notice appear in all copies. > + * > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > + */ > + > +#include > +#include > + > +#include "b64.h" > + > +static const uint8_t b64decode_tbl[] = { > + 0x3e, 0xff, 0xff, 0xff, 0x3f, 0x34, 0x35, 0x36, > + 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0xff, > + 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x01, > + 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, > + 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, > + 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, > + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x1a, 0x1b, > + 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23, > + 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, > + 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x32, 0x33 > +}; > + > +size_t b64decode(void *out, const void *in, size_t len) > +{ > + uint8_t *o = (uint8_t *) out; > + const uint8_t *data = (const uint8_t *) in; > + size_t lenout, i, j; > + > + lenout = b64_decode_size(in, len); > + if (!lenout) > + return 0; > + > + uint32_t cv = 0; > + for (i = 0; i < (len - 4); i += 4) { > + cv = 0; > + for (j = 0; j < 4; j++) { > + uint8_t c = data[i + j] - 43; > + if (c > 79 || (c = b64decode_tbl[c]) == 0xff) > + return 0; > + > + cv |= c; > + if (j != 3) > + cv <<= 6; > + } > + > + o[2] = (uint8_t)(cv & 0xff); > + o[1] = (uint8_t)((cv >> 8) & 0xff); > + o[0] = (uint8_t)((cv >> 16) & 0xff); > + o += 3; > + } > + > + if (data[len - 1] != '=') > + o[2] = (uint8_t)(cv & 0xff); > + > + if (data[len - 1] != '=') > + o[1] = (uint8_t)((cv >> 8) & 0xff); > + > + o[0] = (uint8_t)((cv >> 16) & 0xff); > + > + return lenout; > +} > + > +static const uint8_t b64encode_tbl[] = > + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; > + > +size_t b64encode(void *out, const void *in, size_t len) > +{ > + uint8_t *o = (uint8_t *) out; > + const uint8_t *data = (const uint8_t *) in; > + size_t lenout, pad, i; > + > + lenout = b64_encode_size(len); > + if (!lenout) > + return 0; > + > + for (i = 0; i < len; i += 3) { > + uint32_t cv = (data[i] << 16) | (d
[OpenWrt-Devel] [PATCH] [libubox][v3] b64: add base64 support
The base code has been taken from zstream project which was written by Steven Barth. Signed-off-by: Luka Perkov CC: Steven Barth --- => changes in v2: Use new API: size_t b64decode(void **out, const char *in, size_t len); size_t b64encode(char **out, const void *in, size_t len); => changes in v3: Use new API: static inline int b64_decode_size(const void *in, size_t len); static inline int b64_encode_size(size_t len); size_t b64decode(void *out, const void *in, size_t len); size_t b64encode(void *out, const void *in, size_t len); In this set a few corner cases of invalid writes in b64decode were fixed. Furthermore, b64decode() works fine when in == out. CMakeLists.txt | 2 +- b64.c | 112 + b64.h | 60 +++ 3 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 b64.c create mode 100644 b64.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 58381da..77f4842 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,7 +14,7 @@ IF(JSONC_FOUND) INCLUDE_DIRECTORIES(${JSONC_INCLUDE_DIRS}) ENDIF() -SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c) +SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c b64.c) ADD_LIBRARY(ubox SHARED ${SOURCES}) ADD_LIBRARY(ubox-static STATIC ${SOURCES}) diff --git a/b64.c b/b64.c new file mode 100644 index 000..007fb91 --- /dev/null +++ b/b64.c @@ -0,0 +1,112 @@ +/* + * Copyright (C) 2011 Steven Barth + * Copyright (C) 2015 Luka Perkov + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include +#include + +#include "b64.h" + +static const uint8_t b64decode_tbl[] = { + 0x3e, 0xff, 0xff, 0xff, 0x3f, 0x34, 0x35, 0x36, + 0x37, 0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0xff, + 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x01, + 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, + 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, + 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x1a, 0x1b, + 0x1c, 0x1d, 0x1e, 0x1f, 0x20, 0x21, 0x22, 0x23, + 0x24, 0x25, 0x26, 0x27, 0x28, 0x29, 0x2a, 0x2b, + 0x2c, 0x2d, 0x2e, 0x2f, 0x30, 0x31, 0x32, 0x33 +}; + +size_t b64decode(void *out, const void *in, size_t len) +{ + uint8_t *o = (uint8_t *) out; + const uint8_t *data = (const uint8_t *) in; + size_t lenout, i, j; + + lenout = b64_decode_size(in, len); + if (!lenout) + return 0; + + uint32_t cv = 0; + for (i = 0; i < (len - 4); i += 4) { + cv = 0; + for (j = 0; j < 4; j++) { + uint8_t c = data[i + j] - 43; + if (c > 79 || (c = b64decode_tbl[c]) == 0xff) + return 0; + + cv |= c; + if (j != 3) + cv <<= 6; + } + + o[2] = (uint8_t)(cv & 0xff); + o[1] = (uint8_t)((cv >> 8) & 0xff); + o[0] = (uint8_t)((cv >> 16) & 0xff); + o += 3; + } + + if (data[len - 1] != '=') + o[2] = (uint8_t)(cv & 0xff); + + if (data[len - 1] != '=') + o[1] = (uint8_t)((cv >> 8) & 0xff); + + o[0] = (uint8_t)((cv >> 16) & 0xff); + + return lenout; +} + +static const uint8_t b64encode_tbl[] = + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +size_t b64encode(void *out, const void *in, size_t len) +{ + uint8_t *o = (uint8_t *) out; + const uint8_t *data = (const uint8_t *) in; + size_t lenout, pad, i; + + lenout = b64_encode_size(len); + if (!lenout) + return 0; + + for (i = 0; i < len; i += 3) { + uint32_t cv = (data[i] << 16) | (data[i + 1] << 8) | data[i + 2]; + o[3] = b64encode_tbl[cv & 0x3f]; + o[2] = b64encode_tbl[(cv >> 6) & 0x3f]; + o[1] = b64encode_tbl[(cv >> 12) & 0x3f]; + o[0] = b64encode_tbl[(cv >> 18) & 0x3f]; +