diff --git a/.travis.yml b/.travis.yml index 7eb7746..8f0ff4b 100644 --- a/.travis.yml +++ b/.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) diff --git a/FUZZER-NOTES.md b/FUZZER-NOTES.md new file mode 100644 index 0000000..7b88238 --- /dev/null +++ b/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. diff --git a/Makefile.in b/Makefile.in index 479e188..be2d39e 100644 --- a/Makefile.in +++ b/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" >> $@ diff --git a/common-kex.c b/common-kex.c index 7da3fb7..d4933dd 100644 --- a/common-kex.c +++ b/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(); } diff --git a/common-session.c b/common-session.c index 41bf5b3..96dd4dc 100644 --- a/common-session.c +++ b/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 */ diff --git a/configure.ac b/configure.ac index d1b2602..c0bb8a3 100644 --- a/configure.ac +++ b/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 diff --git a/dbrandom.c b/dbrandom.c index 838f8ca..0a55bc5 100644 --- a/dbrandom.c +++ b/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) diff --git a/dbutil.h b/dbutil.h index 7d1c3e1..7cb9d68 100644 --- a/dbutil.h +++ b/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_ */ diff --git a/ecdsa.h b/ecdsa.h index bb3a18e..01cb134 100644 --- a/ecdsa.h +++ b/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); diff --git a/fuzz-common.c b/fuzz-common.c index f64504f..5c90c45 100644 --- a/fuzz-common.c +++ b/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); +} diff --git a/fuzz-harness.c b/fuzz-harness.c index 00a2ba6..be23d4e 100644 --- a/fuzz-harness.c +++ b/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"); diff --git a/fuzz-wrapfd.c b/fuzz-wrapfd.c index 39d2b62..ed8968a 100644 --- a/fuzz-wrapfd.c +++ b/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) { diff --git a/fuzz.h b/fuzz.h index 9316a0a..dab6c37 100644 --- a/fuzz.h +++ b/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); diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c new file mode 100644 index 0000000..224ff58 --- /dev/null +++ b/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; +} diff --git a/fuzzer-kexecdh.c b/fuzzer-kexecdh.c new file mode 100644 index 0000000..c3a450a --- /dev/null +++ b/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; +} diff --git a/fuzzer-pubkey.c b/fuzzer-pubkey.c index 3dfe7b5..033f496 100644 --- a/fuzzer-pubkey.c +++ b/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); diff --git a/libtommath/bn_fast_s_mp_mul_digs.c b/libtommath/bn_fast_s_mp_mul_digs.c index a1015af..783e7ca 100644 --- a/libtommath/bn_fast_s_mp_mul_digs.c +++ b/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]; } diff --git a/packet.c b/packet.c index 90470ee..cacc06d 100644 --- a/packet.c +++ b/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; } diff --git a/svr-authpubkey.c b/svr-authpubkey.c index 0ca0ea4..ec14ec0 100644 --- a/svr-authpubkey.c +++ b/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. */ diff --git a/svr-authpubkeyoptions.c b/svr-authpubkeyoptions.c index 19f07b9..ba6f698 100644 --- a/svr-authpubkeyoptions.c +++ b/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 == '"') { diff --git a/svr-main.c b/svr-main.c index 6f3144b..0a39b70 100644 --- a/svr-main.c +++ b/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++) { diff --git a/svr-runopts.c b/svr-runopts.c index 2c65009..d6c78df 100644 --- a/svr-runopts.c +++ b/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 */ diff --git a/sysoptions.h b/sysoptions.h index 0028199..942e724 100644 --- a/sysoptions.h +++ b/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 */