Hello tech@,
I gave a look to getint in var.c, which turned out to roll out it's own
version of string to long conversion, without overflow checking.
Attached is a diff to make the function more readable and overflow safe
by using strtol, with error checking based on strtonum.
I choose for returning an error on clamping instead of the clamped
value, because it's dangerous to base calculations on such values, but
this can be easily changed by removing the errno checking.
Sincerely,
Martijn van Duren
Index: var.c
===================================================================
RCS file: /cvs/src/bin/ksh/var.c,v
retrieving revision 1.43
diff -u -p -r1.43 var.c
--- var.c 1 Sep 2015 13:12:31 -0000 1.43
+++ var.c 3 Sep 2015 19:39:44 -0000
@@ -1,11 +1,14 @@
/* $OpenBSD: var.c,v 1.43 2015/09/01 13:12:31 tedu Exp $ */
-#include "sh.h"
+#include <ctype.h>
+#include <errno.h>
+#include <stdlib.h>
#include <time.h>
-#include "ksh_limval.h"
+
#include <sys/stat.h>
-#include <ctype.h>
+#include "ksh_limval.h"
+#include "sh.h"
/*
* Variables
*
@@ -411,11 +414,11 @@ setint(struct tbl *vq, long int n)
int
getint(struct tbl *vp, long int *nump, bool arith)
{
- char *s;
- int c;
- int base, neg;
+ char *s, *endptr;
+ int terrno = errno;
+ int base = 10;
int have_base = 0;
- long num;
+ long int num = 0;
if (vp->flag&SPECIAL)
getspec(vp);
@@ -427,49 +430,41 @@ getint(struct tbl *vp, long int *nump, b
return vp->type;
}
s = vp->val.s + vp->type;
- if (s == NULL) /* redundant given initial test */
- s = null;
- base = 10;
- num = 0;
- neg = 0;
- if (arith && *s == '0' && *(s+1)) {
+
+ if (arith && s[0] == '0' && s[1]) {
s++;
- if (*s == 'x' || *s == 'X') {
+ if (s[0] == 'x' || s[0] == 'X') {
s++;
base = 16;
} else if (vp->flag & ZEROFIL) {
- while (*s == '0')
+ while (s[0] == '0')
s++;
} else
base = 8;
- have_base++;
+ have_base = 1;
}
- for (c = (unsigned char)*s++; c ; c = (unsigned char)*s++) {
- if (c == '-') {
- neg++;
- } else if (c == '#') {
- base = (int) num;
- if (have_base || base < 2 || base > 36)
- return -1;
- num = 0;
- have_base = 1;
- } else if (letnum(c)) {
- if (isdigit(c))
- c -= '0';
- else if (islower(c))
- c -= 'a' - 10; /* todo: assumes ascii */
- else if (isupper(c))
- c -= 'A' - 10; /* todo: assumes ascii */
- else
- c = -1; /* _: force error */
- if (c < 0 || c >= base)
- return -1;
- num = num * base + c;
- } else
- return -1;
+
+ errno = 0;
+ num = strtol(s, &endptr, base);
+ if (s == endptr || (*endptr != '#' && *endptr != '\0') ||
+ errno == ERANGE) {
+ errno = terrno;
+ return -1;
}
- if (neg)
- num = -num;
+ if (*endptr == '\0') {
+ *nump = num;
+ return base;
+ }
+ if (have_base || num < 2 || num > 36)
+ return -1;
+ base = (int) num;
+ s = endptr+1;
+ num = strtol(s, &endptr, base);
+ if (s == endptr || *endptr != '\0' || errno == ERANGE) {
+ errno = terrno;
+ return -1;
+ }
+
*nump = num;
return base;
}
