Re: base64 b64_pton fix
On Mon, Dec 23, 2013 at 02:49:45PM -0500, Ted Unangst wrote: be afraid, be very afraid... b If you base64 encode a buffer that's not a nice multiple of 3, you get padding (=) bytes. When you decode those padding bytes, you should get back a correctly sized buffer, but b64_pton doesn't quite know this. It tries to write past the end sometimes. Test string below. It should decode into a 128 byte buffer, but pton returns -1. p1v3+nehH3N3n+/OokzXpsyGF2VVpxIxkjSn3Mv/Sq74OE1iFuVU+K4bQImuVj S55RB2fpCpbB8Nye7tzrt6h9YPP3yyJfqORDETGmIB4lveZXA4KDxx50F9rYrO dFbTLyWfNBb/8Q2TnD72eY/3Y5P9qwtJwyDL25Tleic8G3g= The fix is to only write to the next byte if we actually need to write something. Reads ok and probably explains a situation I ran into and didn't investigate yet, cool thanks Index: net/base64.c === RCS file: /cvs/src/lib/libc/net/base64.c,v retrieving revision 1.6 diff -u -p -r1.6 base64.c --- net/base64.c 24 Nov 2013 23:51:28 - 1.6 +++ net/base64.c 23 Dec 2013 19:42:23 - @@ -194,6 +194,7 @@ b64_pton(src, target, targsize) size_t targsize; { int tarindex, state, ch; + u_char nextbyte; char *pos; state = 0; @@ -221,22 +222,28 @@ b64_pton(src, target, targsize) break; case 1: if (target) { - if (tarindex + 1 = targsize) + if (tarindex = targsize) return (-1); target[tarindex] |= (pos - Base64) 4; - target[tarindex+1] = ((pos - Base64) 0x0f) - 4 ; + nextbyte = ((pos - Base64) 0x0f) 4; + if (nextbyte tarindex + 1 = targsize) + return (-1); + if (tarindex + 1 targsize) + target[tarindex+1] = nextbyte; } tarindex++; state = 2; break; case 2: if (target) { - if (tarindex + 1 = targsize) + if (tarindex = targsize) return (-1); target[tarindex] |= (pos - Base64) 2; - target[tarindex+1] = ((pos - Base64) 0x03) - 6; + nextbyte = ((pos - Base64) 0x03) 6; + if (nextbyte tarindex + 1 = targsize) + return (-1); + if (tarindex + 1 targsize) + target[tarindex+1] = nextbyte; } tarindex++; state = 3; @@ -292,7 +299,8 @@ b64_pton(src, target, targsize) * zeros. If we don't check them, they become a * subliminal channel. */ - if (target target[tarindex] != 0) + if (target tarindex targsize + target[tarindex] != 0) return (-1); } } else { -- Gilles Chehade https://www.poolp.org @poolpOrg
Re: small ssh refinements
On Mon, Dec 23, 2013 at 10:21:59AM -0500, Ted Unangst wrote: On Mon, Dec 23, 2013 at 10:25, Jérémie Courrèges-Anglas wrote: Ted Unangst t...@tedunangst.com writes: Part one of this diff eliminates a series of u_long casts in favor of just using native size_t types. A few other type adjustments. Wouldn't that make ssh harder to port to systems where those types aren't available? See for example millert's last commit to yacc/skeleton.c. There's two types of porting pain: one-off and constant. Not having a type like ptrdiff_t is the former. It's a problem that can be dealt with once, eg testing for it in configure and typdef'ing as appropriate. Not having %z is in the same boat (although a bit more work to write a test for) since we already have a vsnprintf implementation in the compat library and all the log messages use vsnprintf. Constant pain is when we have to make changes to the portable code and patches no longer apply. These are long-term pain because they keep slowing things down (and, if we get it wrong, cause other problems) so should be weighed up to make sure the benefit is worth the effort. I haven't been through the diff yet to see how much of each type is in there :-) I do note that %z is in SuSv3 (but not v2) so anything not having it is likely to be old but not necessarily obsolete. What are these systems? The counter argument is that using long makes the code harder to port to systems where long and ptrdiff_t are different sizes. And by harder to port, I mean compiles fine but executes incorrectly. I consider that a worse problem. If ptrdiff_t is really out, I think I'd prefer to take my chances with ssize_t being the right size. (there are 3 occurrences of ptrdiff_t in umac.c too). Given that it's already in use and we haven't had any complaints that I'm aware of I have no objection to adding more ptrdiff_t. I'll look through the rest of the diff shortly. -- Darren Tucker (dtucker at zip.com.au) GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69 Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
[PATCH] regress: initial regression tests for usr.bin/indent
usr.bin/indent could use a little love, but first it needs some regression tests. The diff below is my initial attempt at this but before I progress too much further I'd like some feedback on the general direction I'm going, especially that Makefile. Is there a way to generalize all of the dependency rules (i.e. tX: tX.in tX.out tX.expected) so we don't have to manually write one for each REGRESS_TARGET? Also, I apologize in advance if this diff doesn't apply cleanly. I know tech@ strips attachments so I cvs add'ed the /usr/src/regress/usr.bin/indent/ directory and each file inside it. Index: regress/usr.bin/Makefile === RCS file: /work/cvsroot/src/regress/usr.bin/Makefile,v retrieving revision 1.27 diff -p -u -r1.27 Makefile --- regress/usr.bin/Makefile1 Aug 2013 21:26:30 - 1.27 +++ regress/usr.bin/Makefile24 Dec 2013 19:04:06 - @@ -2,7 +2,7 @@ # $NetBSD: Makefile,v 1.1 1997/12/30 23:27:11 cgd Exp $ SUBDIR+= basename bc dc diff diff3 dirname file grep gzip gzsig -SUBDIR+= m4 mandoc sdiff sed sort tsort +SUBDIR+= indent m4 mandoc sdiff sed sort tsort SUBDIR+= xargs .if defined(REGRESS_FULL) Index: regress/usr.bin/indent/Makefile === RCS file: regress/usr.bin/indent/Makefile diff -N regress/usr.bin/indent/Makefile --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/Makefile 24 Dec 2013 19:01:24 - @@ -0,0 +1,39 @@ +# $OpenBSD$ + +REGRESS_TARGETS = t1 t2 t3 t4 + +# .in: input file +# .out: test output +# .expected: expected output + +# t1: blank after declarations (-bad) +# t2: no blank after declarations (-nbad) +# t3: blank after procedures (-bap) +# t4: no blank after procedures (-nbap) + +all: clean regress + +t1.out: t1.in + indent -bad $(*).in $(@) + +t2.out: t2.in + indent -nbad $(*).in $(@) + +t3.out: t3.in + indent -bap $(*).in $(@) + +t4.out: t4.in + indent -nbap $(*).in $(@) + +t1: t1.in t1.out t1.expected +t2: t2.in t2.out t2.expected +t3: t3.in t3.out t3.expected +t4: t4.in t4.out t4.expected +$(REGRESS_TARGETS): + cmp -s $(.CURDIR)/$(@).expected $(.CURDIR)/$(@).out || \ + (echo XXX $(*) failed false) + +clean: + rm -f *.out + +.include bsd.regress.mk Index: regress/usr.bin/indent/t1.expected === RCS file: regress/usr.bin/indent/t1.expected diff -N regress/usr.bin/indent/t1.expected --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/t1.expected 24 Dec 2013 18:54:10 - @@ -0,0 +1,10 @@ +/* $OpenBSD$ */ + +int +main(int argc, char *argv) +{ + int x; + + x = 0; + return x; +} Index: regress/usr.bin/indent/t1.in === RCS file: regress/usr.bin/indent/t1.in diff -N regress/usr.bin/indent/t1.in --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/t1.in24 Dec 2013 18:53:46 - @@ -0,0 +1,3 @@ +/* $OpenBSD$ */ + +int main(int argc,char *argv) {int x;x=0;return x;} Index: regress/usr.bin/indent/t2.expected === RCS file: regress/usr.bin/indent/t2.expected diff -N regress/usr.bin/indent/t2.expected --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/t2.expected 24 Dec 2013 18:54:37 - @@ -0,0 +1,9 @@ +/* $OpenBSD$ */ + +int +main(int argc, char *argv) +{ + int x; + x = 0; + return x; +} Index: regress/usr.bin/indent/t2.in === RCS file: regress/usr.bin/indent/t2.in diff -N regress/usr.bin/indent/t2.in --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/t2.in24 Dec 2013 18:53:34 - @@ -0,0 +1,3 @@ +/* $OpenBSD$ */ + +int main(int argc,char *argv) {int x;x=0;return x;} Index: regress/usr.bin/indent/t3.expected === RCS file: regress/usr.bin/indent/t3.expected diff -N regress/usr.bin/indent/t3.expected --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/t3.expected 24 Dec 2013 18:58:14 - @@ -0,0 +1,11 @@ +/* $OpenBSD$ */ + +int +do_nothing(void) +{ +} +int +main(int argc, char *argv) +{ + return 0; +} Index: regress/usr.bin/indent/t3.in === RCS file: regress/usr.bin/indent/t3.in diff -N regress/usr.bin/indent/t3.in --- /dev/null 1 Jan 1970 00:00:00 - +++ regress/usr.bin/indent/t3.in24 Dec 2013 18:57:53 - @@ -0,0 +1,4 @@ +/* $OpenBSD$ */ + +int do_nothing(void) {} +int main(int argc,char *argv) {return 0;} Index: regress/usr.bin/indent/t4.expected === RCS file: regress/usr.bin/indent/t4.expected diff -N
[PATCH] usr.bin/indent: drop casts on malloc
One more tiny usr.bin/indent diff: drop some casts that are no longer needed. Index: indent.c === RCS file: /work/cvsroot/src/usr.bin/indent/indent.c,v retrieving revision 1.23 diff -p -u -r1.23 indent.c --- indent.c26 Nov 2013 13:21:17 - 1.23 +++ indent.c24 Dec 2013 19:19:10 - @@ -89,10 +89,10 @@ main(int argc, char **argv) ps.last_nl = true; /* this is true if the last thing scanned was * a newline */ ps.last_token = semicolon; -combuf = (char *) malloc(bufsize); -labbuf = (char *) malloc(bufsize); -codebuf = (char *) malloc(bufsize); -tokenbuf = (char *) malloc(bufsize); +combuf = malloc(bufsize); +labbuf = malloc(bufsize); +codebuf = malloc(bufsize); +tokenbuf = malloc(bufsize); if (combuf == NULL || labbuf == NULL || codebuf == NULL || tokenbuf == NULL) err(1, NULL); @@ -109,7 +109,7 @@ main(int argc, char **argv) s_com = e_com = combuf + 1; s_token = e_token = tokenbuf + 1; -in_buffer = (char *) malloc(10); +in_buffer = malloc(10); if (in_buffer == NULL) err(1, NULL); in_buffer_limit = in_buffer + 8; Index: lexi.c === RCS file: /work/cvsroot/src/usr.bin/indent/lexi.c,v retrieving revision 1.16 diff -p -u -r1.16 lexi.c --- lexi.c 26 Nov 2013 13:21:17 - 1.16 +++ lexi.c 24 Dec 2013 19:19:38 - @@ -576,7 +576,7 @@ addkey(char *key, int val) */ nspecials = sizeof (specialsinit) / sizeof (specialsinit[0]); maxspecials = nspecials + (nspecials 2); - specials = (struct templ *)calloc(maxspecials, sizeof specials[0]); + specials = calloc(maxspecials, sizeof specials[0]); if (specials == NULL) err(1, NULL); memcpy(specials, specialsinit, sizeof specialsinit);
[PATCH] usr.bin/indent: add -Wall -Werror to CFLAGS
usr.bin/indent nearly builds clean with -Wall -Werror. The diff below adds both to CFLAGS and fixes the minor fallout. Arguably, this doesn't buy us that much, but it makes me feel a little warmer before I start making too many more changes. I believe initializing tabs_to_var to zero is safe because when tabs_to_var is set on line 894 in indent.c it defaults to 0 when not using tabs anyways: tabs_to_var = (use_tabs ? ps.decl_indent 0 : 0); I believe initializing target_col to one is safe because target_col is set on line 138 in io.c: target_col = compute_code_target(); And compute_code_target(void) initializes target_col to one when the indentation level is zero: int compute_code_target(void) { int target_col; target_col = ps.ind_size * ps.ind_level + 1; ... This diff should be applied in usr.bin/indent/. (I just noticed that RCS file in these diffs is pointing to my local cvsync mirror; will that cause problems for others when trying to apply these diffs?) Index: Makefile === RCS file: /work/cvsroot/src/usr.bin/indent/Makefile,v retrieving revision 1.2 diff -p -u -r1.2 Makefile --- Makefile26 Jun 1996 05:34:27 - 1.2 +++ Makefile24 Dec 2013 19:11:24 - @@ -3,4 +3,6 @@ PROG= indent SRCS= indent.c io.c lexi.c parse.c pr_comment.c args.c +CFLAGS+= -Wall -Werror + .include bsd.prog.mk Index: indent.c === RCS file: /work/cvsroot/src/usr.bin/indent/indent.c,v retrieving revision 1.23 diff -p -u -r1.23 indent.c --- indent.c26 Nov 2013 13:21:17 - 1.23 +++ indent.c24 Dec 2013 19:14:22 - @@ -132,6 +132,7 @@ main(int argc, char **argv) output = 0; +tabs_to_var = 0; /*--*\ Index: io.c === RCS file: /work/cvsroot/src/usr.bin/indent/io.c,v retrieving revision 1.13 diff -p -u -r1.13 io.c --- io.c26 Nov 2013 13:21:17 - 1.13 +++ io.c24 Dec 2013 19:13:51 - @@ -53,6 +53,8 @@ dump_line(void) int cur_col, target_col; static int not_first_line; +target_col = 1; + if (ps.procname[0]) { if (troff) { if (comment_open) {
mke2fs.c memory leak
From NetBSD: free(bbp) in error paths. Coverity CID 274748. Index: src/sbin/newfs_ext2fs/mke2fs.c === RCS file: /cvs/src/sbin/newfs_ext2fs/mke2fs.c,v retrieving revision 1.5 diff -u -p -r1.5 mke2fs.c --- src/sbin/newfs_ext2fs/mke2fs.c 17 Apr 2013 03:33:13 - 1.5 +++ src/sbin/newfs_ext2fs/mke2fs.c 25 Dec 2013 05:52:25 - @@ -1262,8 +1262,10 @@ alloc(uint32_t size, uint16_t mode) #endif loc = skpc(~0U, len, bbp); - if (loc == 0) + if (loc == 0) { + free(bbp); return 0; + } loc = len - loc; map = bbp[loc]; bno = loc * NBBY; @@ -1271,6 +1273,7 @@ alloc(uint32_t size, uint16_t mode) if ((map (1 i)) == 0) goto gotit; } + free(bbp); return 0; gotit: