Re: base64 b64_pton fix

2013-12-24 Thread Gilles Chehade
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

2013-12-24 Thread Darren Tucker
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

2013-12-24 Thread Kent R. Spillner
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

2013-12-24 Thread Kent R. Spillner
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

2013-12-24 Thread Kent R. Spillner
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

2013-12-24 Thread Loganaden Velvindron
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: