From 25a0d84aa49a885d0da533e6b2641e05c65dfa48 Mon Sep 17 00:00:00 2001 From: dtucker Date: Tue, 5 Dec 2017 23:59:47 +0000 Subject: [PATCH] Replace atoi and strtol conversions for integer arguments to config keywords with a checking wrapper around strtonum. This will prevent and flag invalid and negative arguments to these keywords. ok djm@ --- usr.bin/ssh/misc.c | 16 +++++++++++++++- usr.bin/ssh/misc.h | 3 ++- usr.bin/ssh/readconf.c | 16 +++++----------- usr.bin/ssh/servconf.c | 10 +++++----- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/usr.bin/ssh/misc.c b/usr.bin/ssh/misc.c index 547f2bd3226..746d95b4917 100644 --- a/usr.bin/ssh/misc.c +++ b/usr.bin/ssh/misc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.119 2017/11/25 06:46:22 dtucker Exp $ */ +/* $OpenBSD: misc.c,v 1.120 2017/12/05 23:59:47 dtucker Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2005,2006 Damien Miller. All rights reserved. @@ -2026,3 +2026,17 @@ bad: *errstr = errbuf; return 0; } + +const char * +atoi_err(const char *nptr, int *val) +{ + const char *errstr = NULL; + long long num; + + if (nptr == NULL || *nptr == '\0') + return "missing"; + num = strtonum(nptr, 0, INT_MAX, &errstr); + if (errstr == NULL) + *val = (int)num; + return errstr; +} diff --git a/usr.bin/ssh/misc.h b/usr.bin/ssh/misc.h index 8e24d5c4484..a81a124e624 100644 --- a/usr.bin/ssh/misc.h +++ b/usr.bin/ssh/misc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.h,v 1.68 2017/11/25 06:46:22 dtucker Exp $ */ +/* $OpenBSD: misc.h,v 1.69 2017/12/05 23:59:47 dtucker Exp $ */ /* * Author: Tatu Ylonen @@ -74,6 +74,7 @@ double monotime_double(void); void lowercase(char *s); int unix_listener(const char *, int, int); int valid_domain(char *, int, const char **); +const char *atoi_err(const char *, int *); struct passwd *pwcopy(struct passwd *); const char *ssh_gai_strerror(int); diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c index e3a7e110d7a..59f0032c523 100644 --- a/usr.bin/ssh/readconf.c +++ b/usr.bin/ssh/readconf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: readconf.c,v 1.280 2017/10/21 23:06:24 millert Exp $ */ +/* $OpenBSD: readconf.c,v 1.281 2017/12/05 23:59:47 dtucker Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -802,6 +802,7 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host, const struct multistate *multistate_ptr; struct allowed_cname *cname; glob_t gl; + const char *errstr; if (activep == NULL) { /* We are processing a command line directive */ cmdline = 1; @@ -1116,15 +1117,9 @@ parse_command: intptr = &options->port; parse_int: arg = strdelim(&s); - if (!arg || *arg == '\0') - fatal("%.200s line %d: Missing argument.", filename, linenum); - if (arg[0] < '0' || arg[0] > '9') - fatal("%.200s line %d: Bad number.", filename, linenum); - - /* Octal, decimal, or hex format? */ - value = strtol(arg, &endofnumber, 0); - if (arg == endofnumber) - fatal("%.200s line %d: Bad number.", filename, linenum); + if ((errstr = atoi_err(arg, &value)) != NULL) + fatal("%s line %d: integer value %s.", + filename, linenum, errstr); if (*activep && *intptr == -1) *intptr = value; break; @@ -1519,7 +1514,6 @@ parse_keytypes: case oCanonicalDomains: value = options->num_canonical_domains != 0; while ((arg = strdelim(&s)) != NULL && *arg != '\0') { - const char *errstr; if (!valid_domain(arg, 1, &errstr)) { fatal("%s line %d: %s", filename, linenum, errstr); diff --git a/usr.bin/ssh/servconf.c b/usr.bin/ssh/servconf.c index e56dd51cf80..388bc322757 100644 --- a/usr.bin/ssh/servconf.c +++ b/usr.bin/ssh/servconf.c @@ -1,5 +1,5 @@ -/* $OpenBSD: servconf.c,v 1.321 2017/12/05 23:56:07 dtucker Exp $ */ +/* $OpenBSD: servconf.c,v 1.322 2017/12/05 23:59:47 dtucker Exp $ */ /* * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland * All rights reserved @@ -1095,6 +1095,7 @@ process_server_config_line(ServerOptions *options, char *line, size_t len; long long val64; const struct multistate *multistate_ptr; + const char *errstr; /* Strip trailing whitespace. Allow \f (form feed) at EOL only */ if ((len = strlen(line)) == 0) @@ -1378,10 +1379,9 @@ process_server_config_line(ServerOptions *options, char *line, intptr = &options->x11_display_offset; parse_int: arg = strdelim(&cp); - if (!arg || *arg == '\0') - fatal("%s line %d: missing integer value.", - filename, linenum); - value = atoi(arg); + if ((errstr = atoi_err(arg, &value)) != NULL) + fatal("%s line %d: integer value %s.", + filename, linenum, errstr); if (*activep && *intptr == -1) *intptr = value; break; -- 2.20.1