- avsm@cvs.openbsd.org 2005/05/23 22:44:01

     [moduli.c ssh-keygen.c]
     - removes signed/unsigned comparisons in moduli generation
     - use strtonum instead of atoi where its easier
     - check some strlcpy overflow and fatal instead of truncate

3 files changed, 42 insertions, 22 deletions

diff --git a/ChangeLog b/ChangeLog
index 82ab680f0..caf31ec86 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -71,6 +71,11 @@
    - jmc@cvs.openbsd.org 2005/05/20 11:23:32
      [ssh_config.5]
      oops - article and spacing;
+   - avsm@cvs.openbsd.org 2005/05/23 22:44:01
+     [moduli.c ssh-keygen.c]
+     - removes signed/unsigned comparisons in moduli generation
+     - use strtonum instead of atoi where its easier
+     - check some strlcpy overflow and fatal instead of truncate
 
 20050524
  - (djm) [contrib/caldera/openssh.spec contrib/redhat/openssh.spec]
@@ -2570,4 +2575,4 @@
    - (djm) Trim deprecated options from INSTALL. Mention UsePAM
    - (djm) Fix quote handling in sftp; Patch from admorten AT umich.edu
 
-$Id: ChangeLog,v 1.3782 2005/05/26 02:14:32 djm Exp $
+$Id: ChangeLog,v 1.3783 2005/05/26 02:16:18 djm Exp $
diff --git a/moduli.c b/moduli.c
index 8b05248e2..c13c535d6 100644
--- a/moduli.c
+++ b/moduli.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: moduli.c,v 1.10 2005/01/17 03:25:46 dtucker Exp $ */
+/* $OpenBSD: moduli.c,v 1.11 2005/05/23 22:44:01 avsm Exp $ */
 /*
  * Copyright 1994 Phil Karn <karn@qualcomm.com>
  * Copyright 1996-1998, 2003 William Allen Simpson <wsimpson@greendragon.com>
@@ -144,7 +144,7 @@ static u_int32_t *LargeSieve, largewords, largetries, largenumbers;
 static u_int32_t largebits, largememory;        /* megabytes */
 static BIGNUM *largebase;
 
-int gen_candidates(FILE *, int, int, BIGNUM *);
+int gen_candidates(FILE *, u_int32_t, u_int32_t, BIGNUM *);
 int prime_test(FILE *, FILE *, u_int32_t, u_int32_t);
 
 /*
@@ -241,14 +241,15 @@ sieve_large(u_int32_t s)
  * The list is checked against small known primes (less than 2**30).
  */
 int
-gen_candidates(FILE *out, int memory, int power, BIGNUM *start)
+gen_candidates(FILE *out, u_int32_t memory, u_int32_t power, BIGNUM *start)
 {
         BIGNUM *q;
         u_int32_t j, r, s, t;
         u_int32_t smallwords = TINY_NUMBER >> 6;
         u_int32_t tinywords = TINY_NUMBER >> 6;
         time_t time_start, time_stop;
-        int i, ret = 0;
+        u_int32_t i;
+        int ret = 0;
 
         largememory = memory;
 
@@ -548,7 +549,7 @@ prime_test(FILE *in, FILE *out, u_int32_t trials, u_int32_t generator_wanted)
                  * due to earlier inconsistencies in interpretation, check
                  * the proposed bit size.
                  */
-                if (BN_num_bits(p) != (in_size + 1)) {
+                if ((u_int32_t)BN_num_bits(p) != (in_size + 1)) {
                         debug2("%10u: bit size %u mismatch", count_in, in_size);
                         continue;
                 }
diff --git a/ssh-keygen.c b/ssh-keygen.c
index 6f0713dab..bee431242 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -12,7 +12,7 @@
  */
 
 #include "includes.h"
-RCSID("$OpenBSD: ssh-keygen.c,v 1.123 2005/04/05 13:45:31 otto Exp $");
+RCSID("$OpenBSD: ssh-keygen.c,v 1.124 2005/05/23 22:44:01 avsm Exp $");
 
 #include <openssl/evp.h>
 #include <openssl/pem.h>
@@ -36,7 +36,7 @@ RCSID("$OpenBSD: ssh-keygen.c,v 1.123 2005/04/05 13:45:31 otto Exp $");
 #include "dns.h"
 
 /* Number of bits in the RSA/DSA key.  This value can be changed on the command line. */
-int bits = 1024;
+u_int32_t bits = 1024;
 
 /*
  * Flag indicating that we just want to change the passphrase.  This can be
@@ -90,7 +90,7 @@ extern char *__progname;
 char hostname[MAXHOSTNAMELEN];
 
 /* moduli.c */
-int gen_candidates(FILE *, int, int, BIGNUM *);
+int gen_candidates(FILE *, u_int32_t, u_int32_t, BIGNUM *);
 int prime_test(FILE *, FILE *, u_int32_t, u_int32_t);
 
 static void
@@ -1007,8 +1007,8 @@ main(int ac, char **av)
         Key *private, *public;
         struct passwd *pw;
         struct stat st;
-        int opt, type, fd, download = 0, memory = 0;
-        int generator_wanted = 0, trials = 100;
+        int opt, type, fd, download = 0;
+        uint32_t memory = 0, generator_wanted = 0, trials = 100;
         int do_gen_candidates = 0, do_screen_candidates = 0;
         int log_level = SYSLOG_LEVEL_INFO;
         BIGNUM *start = NULL;
@@ -1016,6 +1016,7 @@ main(int ac, char **av)
 
         extern int optind;
         extern char *optarg;
+        const char *errstr;
 
         __progname = ssh_get_progname(av[0]);
 
@@ -1040,9 +1041,9 @@ main(int ac, char **av)
             "degiqpclBHvxXyF:b:f:t:U:D:P:N:C:r:g:R:T:G:M:S:a:W:")) != -1) {
                 switch (opt) {
                 case 'b':
-                        bits = atoi(optarg);
-                        if (bits < 512 || bits > 32768) {
-                                printf("Bits has bad value.\n");
+                        bits = strtonum(optarg, 512, 32768, &errstr);
+                        if (errstr) {
+                                printf("Bits has bad value %s (%s)\n", optarg, errstr);
                                 exit(1);
                         }
                         break;
@@ -1070,7 +1071,9 @@ main(int ac, char **av)
                         change_comment = 1;
                         break;
                 case 'f':
-                        strlcpy(identity_file, optarg, sizeof(identity_file));
+                        if (strlcpy(identity_file, optarg, sizeof(identity_file)) >=
+                            sizeof(identity_file))
+                                fatal("Identity filename too long");
                         have_identity = 1;
                         break;
                 case 'g':
@@ -1125,23 +1128,34 @@ main(int ac, char **av)
                         rr_hostname = optarg;
                         break;
                 case 'W':
-                        generator_wanted = atoi(optarg);
-                        if (generator_wanted < 1)
-                                fatal("Desired generator has bad value.");
+                        generator_wanted = strtonum(optarg, 1, UINT_MAX, &errstr);
+                        if (errstr)
+                                fatal("Desired generator has bad value: %s (%s)",
+                                        optarg, errstr);
                         break;
                 case 'a':
-                        trials = atoi(optarg);
+                        trials = strtonum(optarg, 1, UINT_MAX, &errstr);
+                        if (errstr)
+                                fatal("Invalid number of trials: %s (%s)",
+                                        optarg, errstr);
                         break;
                 case 'M':
-                        memory = atoi(optarg);
+                        memory = strtonum(optarg, 1, UINT_MAX, &errstr);
+                        if (errstr) {
+                                fatal("Memory limit is %s: %s", errstr, optarg);
+                        }
                         break;
                 case 'G':
                         do_gen_candidates = 1;
-                        strlcpy(out_file, optarg, sizeof(out_file));
+                        if (strlcpy(out_file, optarg, sizeof(out_file)) >=
+                            sizeof(out_file))
+                                fatal("Output filename too long");
                         break;
                 case 'T':
                         do_screen_candidates = 1;
-                        strlcpy(out_file, optarg, sizeof(out_file));
+                        if (strlcpy(out_file, optarg, sizeof(out_file)) >=
+                            sizeof(out_file))
+                                fatal("Output filename too long");
                         break;
                 case 'S':
                         /* XXX - also compare length against bits */