Here are some minor defects found with a static analysis tool I have
access to.
E
--
Erik Hovland
mail: [EMAIL PROTECTED]
web: http://hovland.org/
PGP/GPG public key available on request
getc and its ilk return an int. Using char truncates the value.
From: Erik Hovland <[EMAIL PROTECTED]>
---
cli-kex.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/cli-kex.c b/cli-kex.c
index 37de6e3..f03c473 100644
--- a/cli-kex.c
+++ b/cli-kex.c
@@ -116,7 +116,7 @@ static void ask_to_confirm(unsigned char* keyblob, unsigned int keybloblen) {
char* fp = NULL;
FILE *tty = NULL;
- char response = 'z';
+ int response = 'z';
fp = sign_key_fingerprint(keyblob, keybloblen);
if (cli_opts.always_accept_key) {
putenv in glibc can have the memory freed (otherwise it will be leaked).
From: Erik Hovland <[EMAIL PROTECTED]>
2.1 or greater.
---
compat.h | 6 ++++++
configure.in | 2 +-
includes.h | 4 ++++
svr-chansession.c | 1 +
4 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/compat.h b/compat.h
index 1ab344f..36c2a1e 100644
--- a/compat.h
+++ b/compat.h
@@ -53,4 +53,10 @@ void endusershell();
#define _PATH_DEVNULL "/dev/null"
#endif
+#if defined(__GLIBC__) && (__GLIBC__ >= 2) && defined(__GLIBC_MINOR__) && (__GLIBC_MINOR__ >= 2)
+#define PUTENV_FREE(x) m_free(x)
+#else
+#define PUTENV_FREE(x)
+#endif
+
#endif /* _COMPAT_H_ */
diff --git a/configure.in b/configure.in
index 52a75e0..d3749dd 100644
--- a/configure.in
+++ b/configure.in
@@ -209,7 +209,7 @@ AC_ARG_ENABLE(shadow,
# Checks for header files.
AC_HEADER_STDC
AC_HEADER_SYS_WAIT
-AC_CHECK_HEADERS([fcntl.h limits.h netinet/in.h netinet/tcp.h stdlib.h string.h sys/socket.h sys/time.h termios.h unistd.h crypt.h pty.h ioctl.h libutil.h libgen.h inttypes.h stropts.h utmp.h utmpx.h lastlog.h paths.h util.h netdb.h security/pam_appl.h pam/pam_appl.h netinet/in_systm.h])
+AC_CHECK_HEADERS([fcntl.h limits.h netinet/in.h netinet/tcp.h stdlib.h string.h sys/socket.h sys/time.h termios.h unistd.h crypt.h pty.h ioctl.h libutil.h libgen.h inttypes.h stropts.h utmp.h utmpx.h lastlog.h paths.h util.h netdb.h security/pam_appl.h pam/pam_appl.h netinet/in_systm.h features.h])
# Checks for typedefs, structures, and compiler characteristics.
AC_C_CONST
diff --git a/includes.h b/includes.h
index 03cb1cf..f3d211c 100644
--- a/includes.h
+++ b/includes.h
@@ -120,6 +120,10 @@
#include <libgen.h>
#endif
+#ifdef HAVE_FEATURES_H
+#include <features.h>
+#endif
+
#include "libtomcrypt/src/headers/tomcrypt.h"
#include "libtommath/tommath.h"
diff --git a/svr-chansession.c b/svr-chansession.c
index 7cedb8e..cbc3fe0 100644
--- a/svr-chansession.c
+++ b/svr-chansession.c
@@ -1042,4 +1042,5 @@ void addnewvar(const char* param, const char* var) {
if (putenv(newvar) < 0) {
dropbear_exit("environ error");
}
+ PUTENV_FREE(newvar);
}
fill_passwd has a return value. But it doesn't return.
From: Erik Hovland <[EMAIL PROTECTED]>
---
svr-auth.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/svr-auth.c b/svr-auth.c
index 7fb6568..a303c64 100644
--- a/svr-auth.c
+++ b/svr-auth.c
@@ -215,15 +215,16 @@ static int fill_passwd(const char* username) {
m_free(ses.authstate.pw_passwd);
pw = getpwnam(username);
- if (!pw) {
- return;
- }
+ if (!pw)
+ return DROPBEAR_FAILURE;
+
ses.authstate.pw_uid = pw->pw_uid;
ses.authstate.pw_gid = pw->pw_gid;
ses.authstate.pw_name = m_strdup(pw->pw_name);
ses.authstate.pw_dir = m_strdup(pw->pw_dir);
ses.authstate.pw_shell = m_strdup(pw->pw_shell);
ses.authstate.pw_passwd = m_strdup(pw->pw_passwd);
+ return DROPBEAR_SUCCESS;
}
If md5_done doesn't return CRYPT_OK then hs may be uninitialized in
sign_key_md5_fingerprint().
It is unlikely, but listensocks may not have all of its fds filled in.
Setting them to 0 early on makes sure that there is at least a known
quantity there.
From: Erik Hovland <[EMAIL PROTECTED]>
uninitialized.
---
signkey.c | 3 ++-
svr-main.c | 1 +
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/signkey.c b/signkey.c
index c1ef5e2..14cd8d8 100644
--- a/signkey.c
+++ b/signkey.c
@@ -287,7 +287,8 @@ static char * sign_key_md5_fingerprint(unsigned char* keyblob,
/* skip the size int of the string - this is a bit messy */
md5_process(&hs, keyblob, keybloblen);
- md5_done(&hs, hash);
+ if (md5_done(&hs, hash) != CRYPT_OK)
+ return NULL;
/* "md5 hexfingerprinthere\0", each hex digit is "AB:" etc */
buflen = 4 + 3*MD5_HASH_SIZE;
diff --git a/svr-main.c b/svr-main.c
index f7ce221..76ace97 100644
--- a/svr-main.c
+++ b/svr-main.c
@@ -124,6 +124,7 @@ void main_noinetd() {
int childsock;
int childpipe[2];
+ memset(listensocks, 0, sizeof(listensocks));
/* Note: commonsetup() must happen before we daemon()ise. Otherwise
daemon() will chdir("/"), and we won't be able to find local-dir
hostkeys. */
