ssh-keyscan may, under very specific circumstances, be vulnerable to
something akin to a buffer overflow. It's probably impossible to
exploit, though, if only because ssh-keyscan is not usually run on very
large untrusted input files.
ssh-keyscan uses an fgets() wrapper that uses an unsigned int to keep
track of the length of a buffer holding the current line. On machines
with sufficient address space to hold UINT_MAX / 2 bytes, e.g. amd64,
one can fill those bytes. The next call will be realloc(buf, 0), which
free()s the buffer and returns a non-NULL zero-size chunk of memory. The
program will then try to write to some point in memory about UINT_MAX /
2 past this newly-returned chunk.
Test case:
$ while true; do echo -n 'AAAAAAAAAAAAAAAA'; done | ssh-keyscan -f -
I stumbled upon this a while ago; I was trying to solve
https://bugzilla.mindrot.org/show_bug.cgi?id=1565 ("ssh-keyscan doesn't
like comment-lines"). The patch below rips out the fgets() wrapper and
correctly handles both comments and ridiculously long lines.
Joachim
Index: ssh-keyscan.c
==================================================================RCS file:
/usr/cvs/src/src/usr.bin/ssh/ssh-keyscan.c,v
retrieving revision 1.81
diff -u -p -r1.81 ssh-keyscan.c
--- ssh-keyscan.c 9 Jan 2010 23:04:13 -0000 1.81
+++ ssh-keyscan.c 6 Mar 2010 20:33:34 -0000
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keyscan.c,v 1.78 2009/01/22 10:02:34 djm Exp $ */
+/* $OpenBSD: ssh-keyscan.c,v 1.81 2010/01/09 23:04:13 dtucker Exp $ */
/*
* Copyright 1995, 1996 by David Mazieres <dm at lcs.mit.edu>.
*
@@ -99,122 +99,6 @@ typedef struct Connection {
TAILQ_HEAD(conlist, Connection) tq; /* Timeout Queue */
con *fdcon;
-/*
- * This is just a wrapper around fgets() to make it usable.
- */
-
-/* Stress-test. Increase this later. */
-#define LINEBUF_SIZE 16
-
-typedef struct {
- char *buf;
- u_int size;
- int lineno;
- const char *filename;
- FILE *stream;
- void (*errfun) (const char *,...);
-} Linebuf;
-
-static Linebuf *
-Linebuf_alloc(const char *filename, void (*errfun) (const char *,...))
-{
- Linebuf *lb;
-
- if (!(lb = malloc(sizeof(*lb)))) {
- if (errfun)
- (*errfun) ("linebuf (%s): malloc failed\n",
- filename ? filename : "(stdin)");
- return (NULL);
- }
- if (filename) {
- lb->filename = filename;
- if (!(lb->stream = fopen(filename, "r"))) {
- xfree(lb);
- if (errfun)
- (*errfun) ("%s: %s\n", filename, strerror(errno));
- return (NULL);
- }
- } else {
- lb->filename = "(stdin)";
- lb->stream = stdin;
- }
-
- if (!(lb->buf = malloc((lb->size = LINEBUF_SIZE)))) {
- if (errfun)
- (*errfun) ("linebuf (%s): malloc failed\n", lb->filename);
- xfree(lb);
- return (NULL);
- }
- lb->errfun = errfun;
- lb->lineno = 0;
- return (lb);
-}
-
-static void
-Linebuf_free(Linebuf * lb)
-{
- fclose(lb->stream);
- xfree(lb->buf);
- xfree(lb);
-}
-
-#if 0
-static void
-Linebuf_restart(Linebuf * lb)
-{
- clearerr(lb->stream);
- rewind(lb->stream);
- lb->lineno = 0;
-}
-
-static int
-Linebuf_lineno(Linebuf * lb)
-{
- return (lb->lineno);
-}
-#endif
-
-static char *
-Linebuf_getline(Linebuf * lb)
-{
- size_t n = 0;
- void *p;
-
- lb->lineno++;
- for (;;) {
- /* Read a line */
- if (!fgets(&lb->buf[n], lb->size - n, lb->stream)) {
- if (ferror(lb->stream) && lb->errfun)
- (*lb->errfun)("%s: %s\n", lb->filename,
- strerror(errno));
- return (NULL);
- }
- n = strlen(lb->buf);
-
- /* Return it or an error if it fits */
- if (n > 0 && lb->buf[n - 1] == '\n') {
- lb->buf[n - 1] = '\0';
- return (lb->buf);
- }
- if (n != lb->size - 1) {
- if (lb->errfun)
- (*lb->errfun)("%s: skipping incomplete last line\n",
- lb->filename);
- return (NULL);
- }
- /* Double the buffer if we need more space */
- lb->size *= 2;
- if ((p = realloc(lb->buf, lb->size)) == NULL) {
- lb->size /= 2;
- if (lb->errfun)
- (*lb->errfun)("linebuf (%s): realloc failed\n",
- lb->filename);
- return (NULL);
- }
- lb->buf = p;
- }
-}
-
static int
fdlim_get(int hard)
{
@@ -709,8 +593,10 @@ int
main(int argc, char **argv)
{
int debug_flag = 0, log_level = SYSLOG_LEVEL_INFO;
- int opt, fopt_count = 0;
- char *tname;
+ int opt, fopt_count = 0, j;
+ char *tname, *line;
+ size_t i, line_len;
+ FILE *fp;
extern int optind;
extern char *optarg;
@@ -808,20 +694,52 @@ main(int argc, char **argv)
read_wait_nfdset = howmany(maxfd, NFDBITS);
read_wait = xcalloc(read_wait_nfdset, sizeof(fd_mask));
- if (fopt_count) {
- Linebuf *lb;
- char *line;
- int j;
-
- for (j = 0; j < fopt_count; j++) {
- lb = Linebuf_alloc(argv[j], error);
- if (!lb)
+ line = NULL;
+
+ for (j = 0; j < fopt_count; j++) {
+ if (line == NULL)
+ line = xmalloc(line_len = BUFSIZ);
+
+ if ((fp = fopen(argv[j], "r")) == NULL)
+ fatal("%s: %s: %s", __progname, argv[j],
+ strerror(errno));
+
+ i = 0;
+ while (fgets(&line[i], line_len - i, fp)) {
+ /* Read a line */
+ i += strcspn(&line[i], "\n");
+ if (line[i] == '\0' && i == line_len - 1) {
+ if (line_len > SIZE_MAX / 2)
+ fatal("%s: %s: line %.20s too long",
+ __progname, argv[j], line);
+ line = xrealloc(line, line_len *= 2, 1);
+ continue;
+ }
+
+ /* Strip off comments and whitespace at end */
+ for (i = strcspn(line, "#\n");
+ i > 0 && strchr(" \t", line[i - 1]);
+ i--);
+ line[i] = '\0';
+
+ /* Skip empty lines, comments */
+ if (i == 0)
continue;
- while ((line = Linebuf_getline(lb)) != NULL)
- do_host(line);
- Linebuf_free(lb);
+
+ do_host(line);
+
+ i = 0;
}
+
+ if (ferror(fp))
+ fatal("%s: %s: %s", __progname, argv[j],
+ strerror(errno));
+
+ fclose(fp);
}
+
+ if (line)
+ xfree(line);
while (optind < argc)
do_host(argv[optind++]);