# HG changeset patch # User Matt Johnston # Date 1521564722 -28800 # Node ID 96e4c9b2cc00d58cc1f23bb7ca4d8a08569cf82f # Parent 7f2be495dff63d049da60bea299426116dfec84d# Parent a57822db3eac92051625e85e0d499a1c957edfce merge coverity diff -r 7f2be495dff6 -r 96e4c9b2cc00 .travis.yml --- a/.travis.yml Sun Mar 04 15:07:09 2018 +0800 +++ b/.travis.yml Wed Mar 21 00:52:02 2018 +0800 @@ -19,7 +19,7 @@ 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) diff -r 7f2be495dff6 -r 96e4c9b2cc00 FUZZER-NOTES.md --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/FUZZER-NOTES.md Wed Mar 21 00:52:02 2018 +0800 @@ -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. diff -r 7f2be495dff6 -r 96e4c9b2cc00 Makefile.in --- a/Makefile.in Sun Mar 04 15:07:09 2018 +0800 +++ b/Makefile.in Wed Mar 21 00:52:02 2018 +0800 @@ -70,6 +70,8 @@ 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 @@ ## 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 @@ 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_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" >> $@ diff -r 7f2be495dff6 -r 96e4c9b2cc00 common-kex.c --- a/common-kex.c Sun Mar 04 15:07:09 2018 +0800 +++ b/common-kex.c Wed Mar 21 00:52:02 2018 +0800 @@ -694,6 +694,9 @@ /* 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(); } diff -r 7f2be495dff6 -r 96e4c9b2cc00 common-session.c --- a/common-session.c Sun Mar 04 15:07:09 2018 +0800 +++ b/common-session.c Wed Mar 21 00:52:02 2018 +0800 @@ -152,8 +152,9 @@ 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 @@ * 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 @@ return -1; } - FD_ZERO(&fds); + DROPBEAR_FD_ZERO(&fds); /* select since it's a non-blocking fd */ diff -r 7f2be495dff6 -r 96e4c9b2cc00 configure.ac --- a/configure.ac Sun Mar 04 15:07:09 2018 +0800 +++ b/configure.ac Wed Mar 21 00:52:02 2018 +0800 @@ -329,6 +329,8 @@ 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_SUBST(DROPBEAR_FUZZ) +AC_SUBST(CXX) # Checks for header files. AC_HEADER_STDC diff -r 7f2be495dff6 -r 96e4c9b2cc00 dbrandom.c --- a/dbrandom.c Sun Mar 04 15:07:09 2018 +0800 +++ b/dbrandom.c Wed Mar 21 00:52:02 2018 +0800 @@ -88,7 +88,7 @@ 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) diff -r 7f2be495dff6 -r 96e4c9b2cc00 dbutil.h --- a/dbutil.h Sun Mar 04 15:07:09 2018 +0800 +++ b/dbutil.h Wed Mar 21 00:52:02 2018 +0800 @@ -88,4 +88,11 @@ 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_ */ diff -r 7f2be495dff6 -r 96e4c9b2cc00 ecdsa.h --- a/ecdsa.h Sun Mar 04 15:07:09 2018 +0800 +++ b/ecdsa.h Wed Mar 21 00:52:02 2018 +0800 @@ -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); diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzz-common.c --- a/fuzz-common.c Sun Mar 04 15:07:09 2018 +0800 +++ b/fuzz-common.c Wed Mar 21 00:52:02 2018 +0800 @@ -22,6 +22,7 @@ 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 @@ 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); +} diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzz-harness.c --- a/fuzz-harness.c Sun Mar 04 15:07:09 2018 +0800 +++ b/fuzz-harness.c Wed Mar 21 00:52:02 2018 +0800 @@ -9,6 +9,7 @@ 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 @@ #endif } + int old_fuzz_wrapfds = 0; for (i = 1; i < argc; i++) { if (argv[i][0] == '-') { /* ignore arguments */ @@ -28,11 +30,16 @@ 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"); diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzz-wrapfd.c --- a/fuzz-wrapfd.c Sun Mar 04 15:07:09 2018 +0800 +++ b/fuzz-wrapfd.c Wed Mar 21 00:52:02 2018 +0800 @@ -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 @@ nset++; } } - FD_ZERO(readfds); + DROPBEAR_FD_ZERO(readfds); if (nset > 0) { /* set one */ @@ -222,7 +224,7 @@ nset++; } } - FD_ZERO(writefds); + DROPBEAR_FD_ZERO(writefds); /* set one */ if (nset > 0) { diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzz.h --- a/fuzz.h Sun Mar 04 15:07:09 2018 +0800 +++ b/fuzz.h Wed Mar 21 00:52:02 2018 +0800 @@ -19,6 +19,7 @@ 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); diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzzer-kexdh.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/fuzzer-kexdh.c Wed Mar 21 00:52:02 2018 +0800 @@ -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; +} diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzzer-kexecdh.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/fuzzer-kexecdh.c Wed Mar 21 00:52:02 2018 +0800 @@ -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; +} diff -r 7f2be495dff6 -r 96e4c9b2cc00 fuzzer-pubkey.c --- a/fuzzer-pubkey.c Sun Mar 04 15:07:09 2018 +0800 +++ b/fuzzer-pubkey.c Wed Mar 21 00:52:02 2018 +0800 @@ -20,19 +20,29 @@ 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) { + 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"); + } - if (setjmp(fuzz.jmp) == 0) { - fuzz_checkpubkey_line(fuzz.input, 5, "/home/me/authorized_keys", - algoname, strlen(algoname), - (unsigned char*)keyblob, strlen(keyblob)); + 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); diff -r 7f2be495dff6 -r 96e4c9b2cc00 libtommath/bn_fast_s_mp_mul_digs.c --- a/libtommath/bn_fast_s_mp_mul_digs.c Sun Mar 04 15:07:09 2018 +0800 +++ b/libtommath/bn_fast_s_mp_mul_digs.c Wed Mar 21 00:52:02 2018 +0800 @@ -87,7 +87,7 @@ { 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]; } diff -r 7f2be495dff6 -r 96e4c9b2cc00 packet.c --- a/packet.c Sun Mar 04 15:07:09 2018 +0800 +++ b/packet.c Wed Mar 21 00:52:02 2018 +0800 @@ -364,9 +364,11 @@ #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; } diff -r 7f2be495dff6 -r 96e4c9b2cc00 svr-authpubkey.c --- a/svr-authpubkey.c Sun Mar 04 15:07:09 2018 +0800 +++ b/svr-authpubkey.c Wed Mar 21 00:52:02 2018 +0800 @@ -167,6 +167,10 @@ 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 @@ 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. */ diff -r 7f2be495dff6 -r 96e4c9b2cc00 svr-authpubkeyoptions.c --- a/svr-authpubkeyoptions.c Sun Mar 04 15:07:09 2018 +0800 +++ b/svr-authpubkeyoptions.c Wed Mar 21 00:52:02 2018 +0800 @@ -113,7 +113,6 @@ m_free(ses.authstate.pubkey_options->forced_command); } m_free(ses.authstate.pubkey_options); - ses.authstate.pubkey_options = NULL; } } @@ -169,6 +168,12 @@ 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 == '"') { diff -r 7f2be495dff6 -r 96e4c9b2cc00 svr-main.c --- a/svr-main.c Sun Mar 04 15:07:09 2018 +0800 +++ b/svr-main.c Wed Mar 21 00:52:02 2018 +0800 @@ -178,7 +178,7 @@ /* incoming connection select loop */ for(;;) { - FD_ZERO(&fds); + DROPBEAR_FD_ZERO(&fds); /* listening sockets */ for (i = 0; i < listensockcount; i++) { diff -r 7f2be495dff6 -r 96e4c9b2cc00 svr-runopts.c --- a/svr-runopts.c Sun Mar 04 15:07:09 2018 +0800 +++ b/svr-runopts.c Wed Mar 21 00:52:02 2018 +0800 @@ -526,8 +526,10 @@ 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 @@ #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 @@ #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 */ -#if DROPBEAR_ECDSA + /* 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) { + || svr_opts.hostkey->ecckey256 +#endif +#if DROPBEAR_ECC_384 + || 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); - } else { - any_keys = 1; } #endif - #if DROPBEAR_ECC_384 - if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 384) - && !svr_opts.hostkey->ecckey384) { + if (!svr_opts.hostkey->ecckey384 + && (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 384 )) { disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP384); - } else { - any_keys = 1; } #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 */ diff -r 7f2be495dff6 -r 96e4c9b2cc00 sysoptions.h --- a/sysoptions.h Sun Mar 04 15:07:09 2018 +0800 +++ b/sysoptions.h Wed Mar 21 00:52:02 2018 +0800 @@ -318,4 +318,15 @@ #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 */