merge coverity

--HG--
branch : coverity

.travis.yml

@@ -19,7 +19,7 @@ addons:
 install:
   - autoconf 
   - autoheader 
-  - ./configure $CONFIGURE_FLAGS CFLAGS="-O2 -Wall -Wno-pointer-sign $WEXTRAFLAGS $EXTRACFLAGS" --prefix="$HOME/inst"
+  - ./configure $CONFIGURE_FLAGS CFLAGS="-O2 -Wall -Wno-pointer-sign $WEXTRAFLAGS $EXTRACFLAGS" --prefix="$HOME/inst" || (cat config.log; exit 1)
   - if [ "$NOWRITEV" = "1" ]; then sed -i -e s/HAVE_WRITEV/DONT_HAVE_WRITEV/ config.h ; fi
   - make -j3 
   # avoid concurrent install, osx/freebsd is racey (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208093)

FUZZER-NOTES.md

@@ -0,0 +1,74 @@
+# Fuzzing Dropbear
+
+Dropbear is process-per-session so it assumes calling `dropbear_exit()`
+is fine at any point to clean up. This makes fuzzing a bit trickier. 
+A few pieces of wrapping infrastructure are used to work around this.
+
+The [libfuzzer](http://llvm.org/docs/LibFuzzer.html#fuzz-target) harness
+expects a long running process to continually run a test function with 
+a string of crafted input. That process should not leak resources or exit.
+
+## longjmp
+
+When dropbear runs in fuzz mode it sets up a 
+[`setjmp()`](http://man7.org/linux/man-pages/man3/setjmp.3.html) target prior 
+to launching the code to be fuzzed, and then [`dropbear_exit()`](dbutil.c#L125)
+calls `longjmp()` back there. This avoids exiting though it doesn't free 
+memory or other resources.
+
+## malloc Wrapper
+
+Dropbear normally uses a [`m_malloc()`](dbmalloc.c) function that is the same as `malloc()` but
+exits if allocation fails. In fuzzing mode this is replaced with a tracking allocator
+that stores all allocations in a linked list. After the `longjmp()` occurs the fuzzer target
+calls [`m_malloc_free_epoch(1, 1)`](dbmalloc.c) to clean up any unreleased memory.
+
+If the fuzz target runs to completion it calls `m_malloc_free_epoch(1, 0)` which will reset 
+the tracked allocations but will not free memory - that allows libfuzzer's leak checking
+to detect leaks in normal operation.
+
+## File Descriptor Input
+
+As a network process Dropbear reads and writes from a socket. The wrappers for
+`read()`/`write()`/`select()` in [fuzz-wrapfd.c](fuzz-wrapfd.c) will read from the
+fuzzer input that has been set up with `wrapfd_add()`. `write()` output is
+currently discarded.
+These also test error paths such as EINTR and short reads with certain probabilities.
+
+This allows running the entire dropbear server process with network input provided by the
+fuzzer, without many modifications to the main code. At the time of writing this 
+only runs the pre-authentication stages, though post-authentication could be run similarly.
+
+## Encryption and Randomness
+
+When running in fuzzing mode Dropbear uses a [fixed seed](dbrandom.c#L185)
+every time so that failures can be reproduced. 
+
+Since the fuzzer cannot generate valid encrypted input the packet decryption and
+message authentication calls are disabled, see [packet.c](packet.c). 
+MAC failures are set to occur with a low probability to test that error path.
+
+## Fuzzers
+
+Current fuzzers are
+
+- [fuzzer-preauth](fuzzer-preauth.c) - the fuzzer input is treated as a stream of session input. This will
+  test key exchange, packet ordering, authentication attempts etc.
+
+- [fuzzer-preauth_nomaths](fuzzer-preauth_nomaths.c) - the same as fuzzer-preauth but with asymmetric crypto
+  routines replaced with dummies for faster runtime. corpora are shared 
+  between fuzzers by [oss-fuzz](https://github.com/google/oss-fuzz) so this 
+  will help fuzzer-preauth too.
+
+- [fuzzer-verify](fuzzer-verify.c) - read a key and signature from fuzzer input and verify that signature. 
+  It would not be expected to pass, though some keys with bad parameters are 
+  able to validate with a trivial signature - extra checks are added for that.
+
+- [fuzzer-pubkey](fuzzer-pubkey.c) - test parsing of an `authorized_keys` line.
+
+- [fuzzer-kexdh](fuzzer-kexdh.c) - test Diffie-Hellman key exchange where the fuzz input is the 
+  ephemeral public key that would be received over the network. This is testing `mp_expt_mod()`
+  and and other libtommath routines.
+
+- [fuzzer-kexecdh](fuzzer-kexecdh.c) - test Elliptic Curve Diffie-Hellman key exchange like fuzzer-kexdh.
+  This is testing libtommath ECC routines.

Makefile.in

@@ -70,6 +70,8 @@ ifeq (@DROPBEAR_FUZZ@, 1)
 	dbclientobjs=$(allobjs) cli-main.o
 	dropbearkeyobjs=$(allobjs) $(KEYOBJS)
 	dropbearconvertobjs=$(allobjs) $(CONVERTOBJS)
+	# CXX only set when fuzzing
+	CXX=@CXX@
 else
 	dropbearobjs=$(COMMONOBJS) $(CLISVROBJS) $(SVROBJS)
 	dbclientobjs=$(COMMONOBJS) $(CLISVROBJS) $(CLIOBJS)
@@ -253,7 +255,7 @@ tidy:
 ## Fuzzing targets
 
 # list of fuzz targets
-FUZZ_TARGETS=fuzzer-preauth fuzzer-pubkey fuzzer-verify fuzzer-preauth_nomaths
+FUZZ_TARGETS=fuzzer-preauth fuzzer-pubkey fuzzer-verify fuzzer-preauth_nomaths fuzzer-kexdh fuzzer-kexecdh
 
 FUZZER_OPTIONS = $(addsuffix .options, $(FUZZ_TARGETS))
 
@@ -268,7 +270,7 @@ fuzzstandalone: fuzz-harness.o fuzz-targets
 svrfuzzobjs=$(subst svr-main.o, ,$(dropbearobjs))
 
 # build all the fuzzers. This will require fail to link unless built with
-# make fuzz-targetsk FUZZLIB=-lFuzzer.a 
+# make fuzz-targets FUZZLIB=-lFuzzer.a 
 # or similar - the library provides main().
 fuzz-targets: $(FUZZ_TARGETS) $(FUZZER_OPTIONS)
 
@@ -278,13 +280,18 @@ fuzzer-preauth: fuzzer-preauth.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobj
 fuzzer-preauth_nomaths: fuzzer-preauth_nomaths.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs)
 	$(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@
 
-
 fuzzer-pubkey: fuzzer-pubkey.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs)
 	$(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@
 
 fuzzer-verify: fuzzer-verify.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs)
 	$(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@
 
+fuzzer-kexdh: fuzzer-kexdh.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs)
+	$(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@
+
+fuzzer-kexecdh: fuzzer-kexecdh.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs)
+	$(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@
+
 fuzzer-%.options: Makefile
 	echo "[libfuzzer]"               > $@
 	echo "max_len = 50000"          >> $@

common-kex.c

@@ -694,6 +694,9 @@ void kexecdh_comb_key(struct kex_ecdh_param *param, buffer *pub_them,
 	/* K, the shared secret */
 	buf_putmpint(ses.kexhashbuf, ses.dh_K);
 
+	ecc_free(Q_them);
+	m_free(Q_them);
+
 	/* calculate the hash H to sign */
 	finish_kexhashbuf();
 }

common-session.c

@@ -152,8 +152,9 @@ void session_loop(void(*loophandler)(void)) {
 
 		timeout.tv_sec = select_timeout();
 		timeout.tv_usec = 0;
-		FD_ZERO(&writefd);
-		FD_ZERO(&readfd);
+		DROPBEAR_FD_ZERO(&writefd);
+		DROPBEAR_FD_ZERO(&readfd);
+
 		dropbear_assert(ses.payload == NULL);
 
 		/* We get woken up when signal handlers write to this pipe.
@@ -204,8 +205,8 @@ void session_loop(void(*loophandler)(void)) {
 			 * want to iterate over channels etc for reading, to handle
 			 * server processes exiting etc. 
 			 * We don't want to read/write FDs. */
-			FD_ZERO(&writefd);
-			FD_ZERO(&readfd);
+			DROPBEAR_FD_ZERO(&writefd);
+			DROPBEAR_FD_ZERO(&readfd);
 		}
 		
 		/* We'll just empty out the pipe if required. We don't do
@@ -406,7 +407,7 @@ static int ident_readln(int fd, char* buf, int count) {
 		return -1;
 	}
 
-	FD_ZERO(&fds);
+	DROPBEAR_FD_ZERO(&fds);
 
 	/* select since it's a non-blocking fd */
 	

configure.ac

@@ -329,6 +329,8 @@ AC_ARG_ENABLE(fuzz,
 		AC_DEFINE(DROPBEAR_FUZZ, 1, Fuzzing)
 		AC_MSG_NOTICE(Enabling fuzzing)
 		DROPBEAR_FUZZ=1
+		# libfuzzer needs linking with c++ libraries
+		AC_PROG_CXX
 	],
 	[
 		AC_DEFINE(DROPBEAR_FUZZ, 0, Fuzzing)
@@ -337,6 +339,7 @@ AC_ARG_ENABLE(fuzz,
 
 )
 AC_SUBST(DROPBEAR_FUZZ)
+AC_SUBST(CXX)
 
 # Checks for header files.
 AC_HEADER_STDC

dbrandom.c

@@ -88,7 +88,7 @@ process_file(hash_state *hs, const char *filename,
  			timeout.tv_sec  = 2;
  			timeout.tv_usec = 0;
 
-			FD_ZERO(&read_fds);
+			DROPBEAR_FD_ZERO(&read_fds);
 			FD_SET(readfd, &read_fds);
 			res = select(readfd + 1, &read_fds, NULL, NULL, &timeout);
 			if (res == 0)

dbutil.h

@@ -88,4 +88,11 @@ char * expand_homedir_path(const char *inpath);
 
 void fsync_parent_dir(const char* fn);
 
+#if DROPBEAR_MSAN
+/* FD_ZERO seems to leave some memory uninitialized. clear it to avoid false positives */
+#define DROPBEAR_FD_ZERO(fds) do { memset((fds), 0x0, sizeof(fd_set)); FD_ZERO(fds); } while(0)
+#else
+#define DROPBEAR_FD_ZERO(fds) FD_ZERO(fds)
+#endif
+
 #endif /* DROPBEAR_DBUTIL_H_ */

ecdsa.h

@@ -16,7 +16,7 @@
 #elif DROPBEAR_ECC_521
 #define ECDSA_DEFAULT_SIZE 521
 #else
-#define ECDSA_DEFAULT_SIZE 0
+#error ECDSA cannot be enabled without enabling at least one size (256, 384, 521)
 #endif
 
 ecc_key *gen_ecdsa_priv_key(unsigned int bit_size);

fuzz-common.c

@@ -22,6 +22,7 @@ void fuzz_common_setup(void) {
     fuzz.input = m_malloc(sizeof(buffer));
     _dropbear_log = fuzz_dropbear_log;
     crypto_init();
+    fuzz_seed();
     /* let any messages get flushed */
     setlinebuf(stdout);
 }
@@ -188,3 +189,13 @@ int fuzz_run_preauth(const uint8_t *Data, size_t Size, int skip_kexmaths) {
 
     return 0;
 }
+
+const void* fuzz_get_algo(const algo_type *algos, const char* name) {
+    const algo_type *t;
+    for (t = algos; t->name; t++) {
+        if (strcmp(t->name, name) == 0) {
+            return t->data;
+        }
+    }
+    assert(0);
+}

fuzz-harness.c

@@ -9,6 +9,7 @@ int main(int argc, char ** argv) {
     buffer *input = buf_new(100000);
 
     for (i = 1; i < argc; i++) {
+        printf("arg %s\n", argv[i]);
 #if DEBUG_TRACE
         if (strcmp(argv[i], "-v") == 0) {
             debug_trace = 1;
@@ -17,6 +18,7 @@ int main(int argc, char ** argv) {
 #endif
     }
 
+    int old_fuzz_wrapfds = 0;
     for (i = 1; i < argc; i++) {
         if (argv[i][0] == '-') {
             /* ignore arguments */
@@ -28,11 +30,16 @@ int main(int argc, char ** argv) {
         buf_readfile(input, fn);
         buf_setpos(input, 0);
 
+        fuzz.wrapfds = old_fuzz_wrapfds;
         printf("Running %s once \n", fn);
         LLVMFuzzerTestOneInput(input->data, input->len);
         printf("Running %s twice \n", fn);
         LLVMFuzzerTestOneInput(input->data, input->len);
         printf("Done %s\n", fn);
+
+        /* Disable wrapfd so it won't interfere with buf_readfile() above */
+        old_fuzz_wrapfds = fuzz.wrapfds;
+        fuzz.wrapfds = 0;
     }
 
     printf("Finished\n");

fuzz-wrapfd.c

@@ -2,16 +2,18 @@
 #include "includes.h"
 #include "fuzz-wrapfd.h"
 
+#include "dbutil.h"
+
 #include "fuzz.h"
 
 #define IOWRAP_MAXFD (FD_SETSIZE-1)
 static const int MAX_RANDOM_IN = 50000;
-static const double CHANCE_CLOSE = 1.0 / 300;
-static const double CHANCE_INTR = 1.0 / 200;
-static const double CHANCE_READ1 = 0.6;
-static const double CHANCE_READ2 = 0.3;
-static const double CHANCE_WRITE1 = 0.8;
-static const double CHANCE_WRITE2 = 0.3;
+static const double CHANCE_CLOSE = 1.0 / 600;
+static const double CHANCE_INTR = 1.0 / 900;
+static const double CHANCE_READ1 = 0.96;
+static const double CHANCE_READ2 = 0.5;
+static const double CHANCE_WRITE1 = 0.96;
+static const double CHANCE_WRITE2 = 0.5;
 
 struct fdwrap {
 	enum wrapfd_mode mode;
@@ -195,7 +197,7 @@ int wrapfd_select(int nfds, fd_set *readfds, fd_set *writefds,
 				nset++;
 			}
 		}
-		FD_ZERO(readfds);
+		DROPBEAR_FD_ZERO(readfds);
 
 		if (nset > 0) {
 			/* set one */
@@ -222,7 +224,7 @@ int wrapfd_select(int nfds, fd_set *readfds, fd_set *writefds,
 				nset++;
 			}
 		}
-		FD_ZERO(writefds);
+		DROPBEAR_FD_ZERO(writefds);
 
 		/* set one */
 		if (nset > 0) {

fuzz.h

@@ -19,6 +19,7 @@ void fuzz_svr_setup(void);
 int fuzz_set_input(const uint8_t *Data, size_t Size);
 
 int fuzz_run_preauth(const uint8_t *Data, size_t Size, int skip_kexmaths);
+const void* fuzz_get_algo(const algo_type *algos, const char* name);
 
 // fuzzer functions that intrude into general code
 void fuzz_kex_fakealgos(void);

fuzzer-kexdh.c

@@ -0,0 +1,76 @@
+#include "fuzz.h"
+#include "session.h"
+#include "fuzz-wrapfd.h"
+#include "debug.h"
+#include "runopts.h"
+#include "algo.h"
+#include "bignum.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
+	static int once = 0;
+	static struct key_context* keep_newkeys = NULL;
+	/* number of generated parameters is limited by the timeout for the first run.
+	   TODO move this to the libfuzzer initialiser function instead if the timeout
+	   doesn't apply there */
+	#define NUM_PARAMS 20
+	static struct kex_dh_param *dh_params[NUM_PARAMS];
+
+	if (!once) {
+		fuzz_common_setup();
+		fuzz_svr_setup();
+
+		keep_newkeys = (struct key_context*)m_malloc(sizeof(struct key_context));
+		keep_newkeys->algo_kex = fuzz_get_algo(sshkex, "diffie-hellman-group14-sha256");
+		keep_newkeys->algo_hostkey = DROPBEAR_SIGNKEY_ECDSA_NISTP256;
+		ses.newkeys = keep_newkeys;
+
+		/* Pre-generate parameters */
+		int i;
+		for (i = 0; i < NUM_PARAMS; i++) {
+			dh_params[i] = gen_kexdh_param();
+		}
+
+		once = 1;
+	}
+
+	if (fuzz_set_input(Data, Size) == DROPBEAR_FAILURE) {
+		return 0;
+	}
+
+	m_malloc_set_epoch(1);
+
+	if (setjmp(fuzz.jmp) == 0) {
+		/* Based on recv_msg_kexdh_init()/send_msg_kexdh_reply() 
+		with DROPBEAR_KEX_NORMAL_DH */
+		ses.newkeys = keep_newkeys;
+
+		/* Choose from the collection of ecdh params */
+		unsigned int e = buf_getint(fuzz.input);
+		struct kex_dh_param * dh_param = dh_params[e % NUM_PARAMS];
+
+		DEF_MP_INT(dh_e);
+		m_mp_init(&dh_e);
+		if (buf_getmpint(fuzz.input, &dh_e) != DROPBEAR_SUCCESS) {
+			dropbear_exit("Bad kex value");
+		}
+
+		ses.kexhashbuf = buf_new(KEXHASHBUF_MAX_INTS);
+		kexdh_comb_key(dh_param, &dh_e, svr_opts.hostkey);
+
+		mp_clear(ses.dh_K);
+		m_free(ses.dh_K);
+		mp_clear(&dh_e);
+
+		buf_free(ses.hash);
+		buf_free(ses.session_id);
+		/* kexhashbuf is freed in kexdh_comb_key */
+
+		m_malloc_free_epoch(1, 0);
+	} else {
+		m_malloc_free_epoch(1, 1);
+		TRACE(("dropbear_exit longjmped"))
+		/* dropbear_exit jumped here */
+	}
+
+	return 0;
+}

fuzzer-kexecdh.c

@@ -0,0 +1,82 @@
+#include "fuzz.h"
+#include "session.h"
+#include "fuzz-wrapfd.h"
+#include "debug.h"
+#include "runopts.h"
+#include "algo.h"
+#include "bignum.h"
+
+int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
+	static int once = 0;
+	static const struct dropbear_kex *ecdh[3]; /* 256, 384, 521 */
+	static struct key_context* keep_newkeys = NULL;
+	/* number of generated parameters is limited by the timeout for the first run */
+	#define NUM_PARAMS 80
+	static struct kex_ecdh_param *ecdh_params[NUM_PARAMS];
+
+	if (!once) {
+		fuzz_common_setup();
+		fuzz_svr_setup();
+
+		/* ses gets zeroed by fuzz_set_input */
+		keep_newkeys = (struct key_context*)m_malloc(sizeof(struct key_context));
+		ecdh[0] = fuzz_get_algo(sshkex, "ecdh-sha2-nistp256");
+		ecdh[1] = fuzz_get_algo(sshkex, "ecdh-sha2-nistp384");
+		ecdh[2] = fuzz_get_algo(sshkex, "ecdh-sha2-nistp521");
+		assert(ecdh[0]);
+		assert(ecdh[1]);
+		assert(ecdh[2]);
+		keep_newkeys->algo_hostkey = DROPBEAR_SIGNKEY_ECDSA_NISTP256;
+		ses.newkeys = keep_newkeys;
+
+		/* Pre-generate parameters */
+		int i;
+		for (i = 0; i < NUM_PARAMS; i++) {
+			ses.newkeys->algo_kex = ecdh[i % 3];
+			ecdh_params[i] = gen_kexecdh_param();
+		}
+
+		once = 1;
+	}
+
+	if (fuzz_set_input(Data, Size) == DROPBEAR_FAILURE) {
+		return 0;
+	}
+
+	m_malloc_set_epoch(1);
+
+	if (setjmp(fuzz.jmp) == 0) {
+		/* Based on recv_msg_kexdh_init()/send_msg_kexdh_reply() 
+		with DROPBEAR_KEX_ECDH */
+		ses.newkeys = keep_newkeys;
+
+		/* random choice of ecdh 256, 384, 521 */
+		unsigned char b = buf_getbyte(fuzz.input);
+		ses.newkeys->algo_kex = ecdh[b % 3];
+
+		/* Choose from the collection of ecdh params */
+		unsigned int e = buf_getint(fuzz.input);
+		struct kex_ecdh_param *ecdh_param = ecdh_params[e % NUM_PARAMS];
+
+		buffer * ecdh_qs = buf_getstringbuf(fuzz.input);
+
+		ses.kexhashbuf = buf_new(KEXHASHBUF_MAX_INTS);
+		kexecdh_comb_key(ecdh_param, ecdh_qs, svr_opts.hostkey);
+
+		mp_clear(ses.dh_K);
+		m_free(ses.dh_K);
+		buf_free(ecdh_qs);
+
+		buf_free(ses.hash);
+		buf_free(ses.session_id);
+		/* kexhashbuf is freed in kexdh_comb_key */
+
+		m_malloc_free_epoch(1, 0);
+	} else {
+		m_malloc_free_epoch(1, 1);
+		TRACE(("dropbear_exit longjmped"))
+		/* dropbear_exit jumped here */
+	}
+
+	return 0;
+}

fuzzer-pubkey.c

@@ -20,19 +20,29 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
 
 	m_malloc_set_epoch(1);
 
-	/* choose a keytype based on input */
-	uint8_t b = 0;
-	size_t i;
-	for (i = 0; i < Size; i++) {
-		b ^= Data[i];
-	}
-	const char* algoname = fuzz_signkey_names[b%DROPBEAR_SIGNKEY_NUM_NAMED];
-	const char* keyblob = "blob"; /* keep short */
-
 	if (setjmp(fuzz.jmp) == 0) {
-		fuzz_checkpubkey_line(fuzz.input, 5, "/home/me/authorized_keys", 
-			algoname, strlen(algoname),
-			(unsigned char*)keyblob, strlen(keyblob));
+		buffer *line = buf_getstringbuf(fuzz.input);
+		buffer *keyblob = buf_getstringbuf(fuzz.input);
+
+		unsigned int algolen;
+		char* algoname = buf_getstring(keyblob, &algolen);
+
+		if (have_algo(algoname, algolen, sshhostkey) == DROPBEAR_FAILURE) {
+			dropbear_exit("fuzzer imagined a bogus algorithm");
+		}
+
+		int ret = fuzz_checkpubkey_line(line, 5, "/home/me/authorized_keys",
+			algoname, algolen,
+			keyblob->data, keyblob->len);
+
+		if (ret == DROPBEAR_SUCCESS) {
+			/* fuzz_checkpubkey_line() should have cleaned up for failure */
+			svr_pubkey_options_cleanup();
+		}
+
+		buf_free(line);
+		buf_free(keyblob);
+		m_free(algoname);
 		m_malloc_free_epoch(1, 0);
 	} else {
 		m_malloc_free_epoch(1, 1);

libtommath/bn_fast_s_mp_mul_digs.c

@@ -87,7 +87,7 @@ int fast_s_mp_mul_digs (mp_int * a, mp_int * b, mp_int * c, int digs)
   {
     mp_digit *tmpc;
     tmpc = c->dp;
-    for (ix = 0; ix < (pa + 1); ix++) {
+    for (ix = 0; ix < pa; ix++) {
       /* now extract the previous digit [below the carry] */
       *tmpc++ = W[ix];
     }

packet.c

@@ -364,9 +364,11 @@ static int checkmac() {
 
 #if DROPBEAR_FUZZ
 	if (fuzz.fuzzing) {
-		/* fail 1 in 2000 times to test error path.
-		   note that mac_bytes is all zero prior to kex, so don't test ==0 ! */
-		unsigned int value = *((unsigned int*)&mac_bytes);
+	 	/* fail 1 in 2000 times to test error path. */
+		unsigned int value = 0;
+		if (mac_size > sizeof(value)) {
+			memcpy(&value, mac_bytes, sizeof(value));
+		}
 		if (value % 2000 == 99) {
 			return DROPBEAR_FAILURE;
 		}

svr-authpubkey.c

@@ -167,6 +167,10 @@ out:
 		sign_key_free(key);
 		key = NULL;
 	}
+	/* Retain pubkey options only if auth succeeded */
+	if (!ses.authstate.authdone) {
+		svr_pubkey_options_cleanup();
+	}
 	TRACE(("leave pubkeyauth"))
 }
 
@@ -197,7 +201,12 @@ static int checkpubkey_line(buffer* line, int line_num, const char* filename,
 
 	if (line->len < MIN_AUTHKEYS_LINE || line->len > MAX_AUTHKEYS_LINE) {
 		TRACE(("checkpubkey_line: bad line length %d", line->len))
-		return DROPBEAR_FAILURE;
+		goto out;
+	}
+
+	if (memchr(line->data, 0x0, line->len) != NULL) {
+		TRACE(("checkpubkey_line: bad line has null char"))
+		goto out;
 	}
 
 	/* compare the algorithm. +3 so we have enough bytes to read a space and some base64 characters too. */

svr-authpubkeyoptions.c

@@ -113,7 +113,6 @@ void svr_pubkey_options_cleanup() {
 			m_free(ses.authstate.pubkey_options->forced_command);
 		}
 		m_free(ses.authstate.pubkey_options);
-		ses.authstate.pubkey_options = NULL;
 	}
 }
 
@@ -169,6 +168,12 @@ int svr_add_pubkey_options(buffer *options_buf, int line_num, const char* filena
 		if (match_option(options_buf, "command=\"") == DROPBEAR_SUCCESS) {
 			int escaped = 0;
 			const unsigned char* command_start = buf_getptr(options_buf, 0);
+
+			if (ses.authstate.pubkey_options->forced_command) {
+				/* multiple command= options */
+				goto bad_option;
+			}
+
 			while (options_buf->pos < options_buf->len) {
 				const char c = buf_getbyte(options_buf);
 				if (!escaped && c == '"') {

svr-main.c

@@ -178,7 +178,7 @@ static void main_noinetd() {
 	/* incoming connection select loop */
 	for(;;) {
 
-		FD_ZERO(&fds);
+		DROPBEAR_FD_ZERO(&fds);
 		
 		/* listening sockets */
 		for (i = 0; i < listensockcount; i++) {

svr-runopts.c

@@ -526,8 +526,10 @@ static void addhostkey(const char *keyfile) {
 
 void load_all_hostkeys() {
 	int i;
-	int disable_unset_keys = 1;
 	int any_keys = 0;
+#ifdef DROPBEAR_ECDSA
+	int loaded_any_ecdsa = 0;
+#endif
 
 	svr_opts.hostkey = new_sign_key();
 
@@ -552,14 +554,8 @@ void load_all_hostkeys() {
 #endif
 	}
 
-#if DROPBEAR_DELAY_HOSTKEY
-	if (svr_opts.delay_hostkey) {
-		disable_unset_keys = 0;
-	}
-#endif
-
 #if DROPBEAR_RSA
-	if (disable_unset_keys && !svr_opts.hostkey->rsakey) {
+	if (!svr_opts.delay_hostkey && !svr_opts.hostkey->rsakey) {
 		disablekey(DROPBEAR_SIGNKEY_RSA);
 	} else {
 		any_keys = 1;
@@ -567,39 +563,54 @@ void load_all_hostkeys() {
 #endif
 
 #if DROPBEAR_DSS
-	if (disable_unset_keys && !svr_opts.hostkey->dsskey) {
+	if (!svr_opts.delay_hostkey && !svr_opts.hostkey->dsskey) {
 		disablekey(DROPBEAR_SIGNKEY_DSS);
 	} else {
 		any_keys = 1;
 	}
 #endif
 
-
 #if DROPBEAR_ECDSA
+	/* We want to advertise a single ecdsa algorithm size.
+	- If there is a ecdsa hostkey at startup we choose that that size.
+	- If we generate at runtime we choose the default ecdsa size.
+	- Otherwise no ecdsa keys will be advertised */
+
+	/* check if any keys were loaded at startup */
+	loaded_any_ecdsa = 
+		0
 #if DROPBEAR_ECC_256
-	if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 256)
-		&& !svr_opts.hostkey->ecckey256) {
-		disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP256);
-	} else {
-		any_keys = 1;
-	}
+		|| svr_opts.hostkey->ecckey256
 #endif
-
 #if DROPBEAR_ECC_384
-	if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 384)
-		&& !svr_opts.hostkey->ecckey384) {
-		disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP384);
-	} else {
-		any_keys = 1;
+		|| svr_opts.hostkey->ecckey384
+#endif
+#if DROPBEAR_ECC_521
+		|| svr_opts.hostkey->ecckey521
+#endif
+		;
+	any_keys |= loaded_any_ecdsa;
+
+	/* Or an ecdsa key could be generated at runtime */
+	any_keys |= svr_opts.delay_hostkey;
+
+	/* At most one ecdsa key size will be left enabled */
+#if DROPBEAR_ECC_256
+	if (!svr_opts.hostkey->ecckey256
+		&& (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 256 )) {
+		disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP256);
+	}
+#endif
+#if DROPBEAR_ECC_384
+	if (!svr_opts.hostkey->ecckey384
+		&& (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 384 )) {
+		disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP384);
 	}
 #endif
-
 #if DROPBEAR_ECC_521
-	if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 521)
-		&& !svr_opts.hostkey->ecckey521) {
+	if (!svr_opts.hostkey->ecckey521
+		&& (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 521 )) {
 		disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP521);
-	} else {
-		any_keys = 1;
 	}
 #endif
 #endif /* DROPBEAR_ECDSA */

sysoptions.h

@@ -318,4 +318,15 @@ If you test it please contact the Dropbear author */
 
 #define DROPBEAR_TRACKING_MALLOC (DROPBEAR_FUZZ)
 
+/* Used to work around Memory Sanitizer false positives */
+#if defined(__has_feature)
+#  if __has_feature(memory_sanitizer)
+#    define DROPBEAR_MSAN 1
+#  endif
+#endif
+#ifndef DROPBEAR_MSAN 
+#define DROPBEAR_MSAN 0
+#endif
+
+
 /* no include guard for this file */