[PATCH 4/5] misc: fix error-checking in ssh_analyze_banner

[Jon Simons <jon@xxxxxxxxxxxxx>]

From c0dd67abd20fa687d270926c105f0da17bc228ec Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Tue, 11 Jul 2017 19:23:39 -0400
Subject: [PATCH 4/5] misc: fix error-checking in ssh_analyze_banner

Fix error-checking for `strtoul` in `ssh_analyze_banner`, and
enable some tests which demonstrate the fix before-and-after.

Signed-off-by: Jon Simons <jon@xxxxxxxxxxxxx>
---
 src/misc.c                     | 25 ++++++++++++++++++-------
 tests/unittests/torture_misc.c |  2 --
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/misc.c b/src/misc.c
index 25663822..21276c68 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -35,6 +35,7 @@
 
 #endif /* _WIN32 */
 
+#include <errno.h>
 #include <limits.h>
 #include <stdio.h>
 #include <string.h>
@@ -845,7 +846,9 @@ int ssh_analyze_banner(ssh_session session, int server, int *ssh1, int *ssh2) {
 
     openssh = strstr(banner, "OpenSSH");
     if (openssh != NULL) {
-        unsigned int major, minor;
+        char *tmp = NULL;
+        unsigned long int major = 0UL;
+        unsigned long int minor = 0UL;
 
         /*
          * The banner is typical:
@@ -853,25 +856,33 @@ int ssh_analyze_banner(ssh_session session, int server, int *ssh1, int *ssh2) {
          * 012345678901234567890
          */
         if (strlen(openssh) > 9) {
-            major = strtoul(openssh + 8, (char **) NULL, 10);
-            if (major < 1 || major > 100) {
+            major = strtoul(openssh + 8, &tmp, 10);
+            if ((tmp == (openssh + 8)) ||
+                ((errno == ERANGE) && (major == ULONG_MAX)) ||
+                ((errno != 0) && (major == 0)) ||
+                ((major < 1) || (major > 100))) {
                 ssh_set_error(session,
                               SSH_FATAL,
                               "Invalid major version number: %s",
                               banner);
                 return -1;
             }
-            minor = strtoul(openssh + 10, (char **) NULL, 10);
-            if (minor > 100) {
+
+            minor = strtoul(openssh + 10, &tmp, 10);
+            if ((tmp == (openssh + 10)) ||
+                ((errno == ERANGE) && (major == ULONG_MAX)) ||
+                ((errno != 0) && (major == 0)) ||
+                (minor > 100)) {
                 ssh_set_error(session,
                               SSH_FATAL,
                               "Invalid minor version number: %s",
                               banner);
                 return -1;
             }
-            session->openssh = SSH_VERSION_INT(major, minor, 0);
+            session->openssh = SSH_VERSION_INT(((int) major), ((int) minor), 0);
+
             SSH_LOG(SSH_LOG_RARE,
-                    "We are talking to an OpenSSH client version: %d.%d (%x)",
+                    "We are talking to an OpenSSH client version: %lu.%lu (%x)",
                     major, minor, session->openssh);
         }
     }
diff --git a/tests/unittests/torture_misc.c b/tests/unittests/torture_misc.c
index 9cbf57f2..f11f4488 100644
--- a/tests/unittests/torture_misc.c
+++ b/tests/unittests/torture_misc.c
@@ -332,12 +332,10 @@ static void torture_ssh_analyze_banner(void **state) {
     assert_server_banner_rejected("SSH-2.0-OpenSSH_X.9p1");
 
     /* OpenSSH banners: bogus minor */
-    #if 0 /* these don't pass */
     reset_banner_test();
     assert_server_banner_rejected("SSH-2.0-OpenSSH_5.Yp1");
     reset_banner_test();
     assert_client_banner_rejected("SSH-2.0-OpenSSH_5.Yp1");
-    #endif /* these don't pass */
 
     /* OpenSSH banners: ssh-keyscan(1) */
     #if 0 /* these don't pass */
-- 
2.13.2