View Issue Details

IDProjectCategoryView StatusLast Update
0000520tcshGeneralpublic2024-05-03 00:59
ReporterMProG10 Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
Status newResolutionopen 
Summary0000520: Possible overflow issue on expressions.
DescriptionI noticed putn1 breaks the conversion if the last bit is set.

On my machine (an ARM-based cellphone),
@ calc = ( 1 << 63 )
raises a @: Badly formed number. error. I inspected why's that, and turns out the conversion yields a ( (left parenthesis) character.

I can tell the program isn't careful on overflow, since none of the variables and functions are typed unsigned, and there doesn't seem to exist any validity checks on overflow.

Typing putn1's argument as unsigned remedied the issue.

I'm not sure if my solution is portable, but is what made the fix. I also provide portability among different character encodings.

Lastly, line 193 on sh.exp.c has a wrong type. On my machine,
@ ret = ( 0 || 1 << 32 )
yields 0.
Additional Informationhttps://github.com/tcsh-org/tcsh/pull/101
TagsNo tags attached.

Activities

MProG10

2024-05-02 16:47

reporter  

sh.decls.h.diff (602 bytes)   
--- a/sh.decls.h
+++ b/sh.decls.h
@@ -379,8 +379,8 @@ extern	void		  mypipe	(int *);
 extern	struct varent 	 *adrof1	(const Char *, struct varent *);
 extern	void		  doset		(Char **, struct command *);
 extern	void		  dolet		(Char **, struct command *);
-extern	Char		 *putn		(tcsh_number_t);
-extern	tcsh_number_t	  getn		(const Char *);
+extern	Char		 *putn		(unsigned tcsh_number_t);
+extern	unsigned tcsh_number_t	  getn		(const Char *);
 extern	Char		 *value1	(Char *, struct varent *);
 extern	void		  setcopy	(const Char *, const Char *, int);
 extern	void		  setv		(const Char *, Char *, int);
sh.decls.h.diff (602 bytes)   
sh.exp.c.diff (3,605 bytes)   
--- a/sh.exp.c
+++ b/sh.exp.c
@@ -63,7 +63,7 @@ static	Char 	  *exp5		(Char ***, int);
 static	Char 	  *exp6		(Char ***, int);
 static	void	   evalav	(Char **);
 static	int	   isa		(Char *, int);
-static	tcsh_number_t  egetn	(const Char *);
+static	unsigned tcsh_number_t  egetn	(const Char *);
 
 #ifdef EDEBUG
 static	void	   etracc	(const char *, const Char *, Char ***);
@@ -186,11 +186,11 @@ expr(Char ***vp)
 tcsh_number_t
 exp0(Char ***vp, int ignore)
 {
-    tcsh_number_t p1 = exp1(vp, ignore);
+    unsigned tcsh_number_t p1 = exp1(vp, ignore);
 
     etraci("exp0 p1", p1, vp);
     while (**vp && eq(**vp, STRor2)) {
-	int p2;
+	unsigned tcsh_number_t p2;
 
 	(*vp)++;
 
@@ -209,11 +209,11 @@ exp0(Char ***vp, int ignore)
 static tcsh_number_t
 exp1(Char ***vp, int ignore)
 {
-    tcsh_number_t p1 = exp2x(vp, ignore);
+    unsigned tcsh_number_t p1 = exp2x(vp, ignore);
 
     etraci("exp1 p1", p1, vp);
     while (**vp && eq(**vp, STRand2)) {
-	tcsh_number_t p2;
+	unsigned tcsh_number_t p2;
 
 	(*vp)++;
 	p2 = compat_expr ?
@@ -233,11 +233,11 @@ exp1(Char ***vp, int ignore)
 static tcsh_number_t
 exp2x(Char ***vp, int ignore)
 {
-    tcsh_number_t p1 = exp2a(vp, ignore);
+    unsigned tcsh_number_t p1 = exp2a(vp, ignore);
 
     etraci("exp2x p1", p1, vp);
     while (**vp && eq(**vp, STRor)) {
-	tcsh_number_t p2;
+	unsigned tcsh_number_t p2;
 
 	(*vp)++;
 	p2 = compat_expr ?
@@ -256,11 +256,11 @@ exp2x(Char ***vp, int ignore)
 static tcsh_number_t
 exp2a(Char ***vp, int ignore)
 {
-    tcsh_number_t p1 = exp2b(vp, ignore);
+    unsigned tcsh_number_t p1 = exp2b(vp, ignore);
 
     etraci("exp2a p1", p1, vp);
     while (**vp && eq(**vp, STRcaret)) {
-	tcsh_number_t p2;
+	unsigned tcsh_number_t p2;
 
 	(*vp)++;
 	p2 = compat_expr ?
@@ -279,11 +279,11 @@ exp2a(Char ***vp, int ignore)
 static tcsh_number_t
 exp2b(Char ***vp, int ignore)
 {
-    tcsh_number_t p1 = exp2c(vp, ignore);
+    unsigned tcsh_number_t p1 = exp2c(vp, ignore);
 
     etraci("exp2b p1", p1, vp);
     while (**vp && eq(**vp, STRand)) {
-	tcsh_number_t p2;
+	unsigned tcsh_number_t p2;
 
 	(*vp)++;
 	p2 = compat_expr ?
@@ -304,7 +304,7 @@ exp2c(Char ***vp, int ignore)
 {
     Char *p1 = exp3(vp, ignore);
     Char *p2;
-    tcsh_number_t i;
+    unsigned tcsh_number_t i;
 
     cleanup_push(p1, xfree);
     etracc("exp2c p1", p1, vp);
@@ -346,7 +346,7 @@ static Char *
 exp3(Char ***vp, int ignore)
 {
     Char *p1, *p2;
-    tcsh_number_t i;
+    unsigned tcsh_number_t i;
 
     p1 = exp3a(vp, ignore);
     etracc("exp3 p1", p1, vp);
@@ -393,7 +393,7 @@ exp3a(Char ***vp, int ignore)
 {
     Char *p1, *p2;
     const Char *op;
-    tcsh_number_t i;
+    unsigned tcsh_number_t i;
 
     p1 = exp4(vp, ignore);
     etracc("exp3a p1", p1, vp);
@@ -421,7 +421,7 @@ static Char *
 exp4(Char ***vp, int ignore)
 {
     Char *p1, *p2;
-    tcsh_number_t i = 0;
+    unsigned tcsh_number_t i = 0;
 
     p1 = exp5(vp, ignore);
     etracc("exp4 p1", p1, vp);
@@ -458,7 +458,7 @@ static Char *
 exp5(Char ***vp, int ignore)
 {
     Char *p1, *p2;
-    tcsh_number_t i = 0;
+    unsigned tcsh_number_t i = 0;
 
     p1 = exp6(vp, ignore);
     etracc("exp5 p1", p1, vp);
@@ -513,8 +513,8 @@ exp5(Char ***vp, int ignore)
 static Char *
 exp6(Char ***vp, int ignore)
 {
-    tcsh_number_t ccode;
-    tcsh_number_t i = 0;
+    unsigned tcsh_number_t ccode,
+			   i = 0;
     Char *cp;
 
     if (**vp == 0)
@@ -1022,7 +1022,7 @@ isa(Char *cp, int what)
     return (0);
 }
 
-static tcsh_number_t
+static unsigned tcsh_number_t
 egetn(const Char *cp)
 {
     if (*cp && *cp != '-' && !Isdigit(*cp))
sh.exp.c.diff (3,605 bytes)   
sh.h.diff (367 bytes)   
--- a/sh.h
+++ b/sh.h
@@ -246,9 +246,9 @@ static __inline void tcsh_ignore(intptr_t a)
 #endif /* __HP_CXD_SPP && !__hpux */
 
 #ifdef HAVE_LONG_LONG
-typedef long long tcsh_number_t;
+#define tcsh_number_t long long
 #else
-typedef long tcsh_number_t;
+#define tcsh_number_t long
 #endif
 /*
  * This macro compares the st_dev field of struct stat. On aix on ibmESA
sh.h.diff (367 bytes)   
sh.set.c.diff (1,978 bytes)   
--- a/sh.set.c
+++ b/sh.set.c
@@ -46,7 +46,7 @@ static	void		 asx		(Char *, int, Char *);
 static	struct varent 	*getvx		(Char *, int);
 static	Char		*xset		(Char *, Char ***);
 static	Char		*operate	(int, Char *, Char *);
-static	void	 	 putn1		(tcsh_number_t);
+static	void	 	 putn1		(unsigned tcsh_number_t);
 static	struct varent	*madrof		(Char *, struct varent *);
 static	void		 unsetv1	(struct varent *);
 static	void		 exportpath	(Char **);
@@ -519,34 +519,47 @@ operate(int op, Char *vp, Char *p)
 static Char *putp;
 
 Char *
-putn(tcsh_number_t n)
+putn(unsigned tcsh_number_t n)
 {
     Char nbuf[1024]; /* Enough even for octal */
 
     putp = nbuf;
-    if (n < 0) {
+    if (n & ~(~(unsigned tcsh_number_t) 0 >> 1)) {
 	n = -n;
 	*putp++ = '-';
     }
     putn1(n);
+#ifdef IS_ASCII
     *putp = 0;
+#endif
     return (Strsave(nbuf));
 }
 
 static void
-putn1(tcsh_number_t n)
+putn1(unsigned tcsh_number_t n)
 {
+#ifndef IS_ASCII
+    sprintf(putp, "%l"
+#ifdef HAVE_LONG_LONG
+    "l"
+#endif /* HAVE_LONG_LONG */
+    "u", n);
+#else
     if (n > 9)
 	putn1(n / 10);
     *putp++ = (Char)(n % 10 + '0');
+#endif /* !IS_ASCII */
 }
 
-tcsh_number_t
+unsigned tcsh_number_t
 getn(const Char *cp)
 {
-    tcsh_number_t n;
+    unsigned tcsh_number_t n;
     int     sign;
     int base;
+#ifndef IS_ASCII
+    char cps[2];
+#endif
 
     if (!cp)			/* PWP: extra error checking */
 	stderror(ERR_NAME | ERR_BADNUM);
@@ -566,12 +579,29 @@ getn(const Char *cp)
     else
 	base = 10;
 
+#ifndef IS_ASCII
+    cps[1] =
+#endif
     n = 0;
     while (Isdigit(*cp))
     {
+#ifndef IS_ASCII
+	unsigned tcsh_number_t ns;
+#endif
+
 	if (base == 8 && *cp >= '8')
 	    stderror(ERR_NAME | ERR_BADNUM);
+#ifndef IS_ASCII
+	cps[0] = *cp++;
+	sscanf(cps, "%l"
+#ifdef HAVE_LONG_LONG
+	"l"
+#endif /* HAVE_LONG_LONG */
+	"u", &ns);
+	n = n * base + ns;
+#else
 	n = n * base + *cp++ - '0';
+#endif /* !IS_ASCII */
     }
     if (*cp)
 	stderror(ERR_NAME | ERR_BADNUM);
sh.set.c.diff (1,978 bytes)   

MProG10

2024-05-03 00:59

reporter   ~0004036

- Type every related part as unsigned;
- Fix relational operators.
sh.set.c-2.diff (778 bytes)   
--- a/sh.set.c
+++ b/sh.set.c
@@ -45,7 +45,7 @@ static	Char		*getinx		(Char *, int *);
 static	void		 asx		(Char *, int, Char *);
 static	struct varent 	*getvx		(Char *, int);
 static	Char		*xset		(Char *, Char ***);
-static	Char		*operate	(int, Char *, Char *);
+static	Char		*operate	(Char, Char *, Char *);
 static	void	 	 putn1		(unsigned tcsh_number_t);
 static	struct varent	*madrof		(Char *, struct varent *);
 static	void		 unsetv1	(struct varent *);
@@ -491,13 +491,13 @@ xset(Char *cp, Char ***vp)
 }
 
 static Char *
-operate(int op, Char *vp, Char *p)
+operate(Char op, Char *vp, Char *p)
 {
     Char    opr[2];
     Char   *vec[5];
     Char **v = vec;
     Char  **vecp = v;
-    tcsh_number_t i;
+    unsigned tcsh_number_t i;
 
     if (op != '=') {
 	if (*vp)
sh.set.c-2.diff (778 bytes)   
sh.decls.h-2.diff (887 bytes)   
--- a/sh.decls.h
+++ b/sh.decls.h
@@ -126,8 +126,8 @@ extern	int		  find_cmd	(Char *, int);
  * sh.exp.c
  */
 extern  Char		 *filetest      (Char *, Char ***, int);
-extern	tcsh_number_t 	  expr		(Char ***);
-extern	tcsh_number_t	  exp0		(Char ***, int);
+extern	unsigned tcsh_number_t 	  expr	(Char ***);
+extern	unsigned tcsh_number_t	  exp0	(Char ***, int);
 
 /*
  * sh.file.c
@@ -380,7 +380,7 @@ extern	struct varent 	 *adrof1	(const Char *, struct varent *);
 extern	void		  doset		(Char **, struct command *);
 extern	void		  dolet		(Char **, struct command *);
 extern	Char		 *putn		(unsigned tcsh_number_t);
-extern	unsigned tcsh_number_t	  getn		(const Char *);
+extern	unsigned tcsh_number_t	  getn	(const Char *);
 extern	Char		 *value1	(Char *, struct varent *);
 extern	void		  setcopy	(const Char *, const Char *, int);
 extern	void		  setv		(const Char *, Char *, int);
sh.decls.h-2.diff (887 bytes)   
sh.exp.c-2.diff (3,612 bytes)   
--- a/sh.exp.c
+++ b/sh.exp.c
@@ -51,11 +51,11 @@
 #define NOTEQMATCH 8
 
 static	int	   sh_access	(const Char *, int);
-static	tcsh_number_t  exp1		(Char ***, int);
-static	tcsh_number_t  exp2x	(Char ***, int);
-static	tcsh_number_t  exp2a	(Char ***, int);
-static	tcsh_number_t  exp2b	(Char ***, int);
-static	tcsh_number_t  exp2c	(Char ***, int);
+static	unsigned tcsh_number_t  exp1	(Char ***, int);
+static	unsigned tcsh_number_t  exp2x	(Char ***, int);
+static	unsigned tcsh_number_t  exp2a	(Char ***, int);
+static	unsigned tcsh_number_t  exp2b	(Char ***, int);
+static	unsigned tcsh_number_t  exp2c	(Char ***, int);
 static	Char 	  *exp3		(Char ***, int);
 static	Char 	  *exp3a	(Char ***, int);
 static	Char 	  *exp4		(Char ***, int);
@@ -177,13 +177,13 @@ sh_access(const Char *fname, int mode)
 #endif /* !POSIX */
 }
 
-tcsh_number_t
+unsigned tcsh_number_t
 expr(Char ***vp)
 {
     return (exp0(vp, 0));
 }
 
-tcsh_number_t
+unsigned tcsh_number_t
 exp0(Char ***vp, int ignore)
 {
     unsigned tcsh_number_t p1 = exp1(vp, ignore);
@@ -206,7 +206,7 @@ exp0(Char ***vp, int ignore)
     return (p1);
 }
 
-static tcsh_number_t
+static unsigned tcsh_number_t
 exp1(Char ***vp, int ignore)
 {
     unsigned tcsh_number_t p1 = exp2x(vp, ignore);
@@ -230,7 +230,7 @@ exp1(Char ***vp, int ignore)
     return (p1);
 }
 
-static tcsh_number_t
+static unsigned tcsh_number_t
 exp2x(Char ***vp, int ignore)
 {
     unsigned tcsh_number_t p1 = exp2a(vp, ignore);
@@ -253,7 +253,7 @@ exp2x(Char ***vp, int ignore)
     return (p1);
 }
 
-static tcsh_number_t
+static unsigned tcsh_number_t
 exp2a(Char ***vp, int ignore)
 {
     unsigned tcsh_number_t p1 = exp2b(vp, ignore);
@@ -276,7 +276,7 @@ exp2a(Char ***vp, int ignore)
     return (p1);
 }
 
-static tcsh_number_t
+static unsigned tcsh_number_t
 exp2b(Char ***vp, int ignore)
 {
     unsigned tcsh_number_t p1 = exp2c(vp, ignore);
@@ -299,7 +299,7 @@ exp2b(Char ***vp, int ignore)
     return (p1);
 }
 
-static tcsh_number_t
+static unsigned tcsh_number_t
 exp2c(Char ***vp, int ignore)
 {
     Char *p1 = exp3(vp, ignore);
@@ -362,21 +362,58 @@ exp3(Char ***vp, int ignore)
 	etracc("exp3 p2", p2, vp);
 	if (!(ignore & TEXP_IGNORE))
 	    switch ((int)i) {
+		tcsh_number_t is;
 
 	    case GTR:
-		i = egetn(p1) > egetn(p2);
+		if ((i = egetn(p1)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    is = -(tcsh_number_t) -i;
+		else
+		    is = i;
+		if ((i = egetn(p2)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    i = is > -(tcsh_number_t) -i;
+		else
+		    i = is > (tcsh_number_t) i;
 		break;
 
 	    case GTR | 1:
-		i = egetn(p1) >= egetn(p2);
+		if ((i = egetn(p1)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    is = -(tcsh_number_t) -i;
+		else
+		    is = i;
+		if ((i = egetn(p2)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    i = is >= -(tcsh_number_t) -i;
+		else
+		    i = is >= (tcsh_number_t) i;
 		break;
 
 	    case LSS:
-		i = egetn(p1) < egetn(p2);
+		if ((i = egetn(p1)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    is = -(tcsh_number_t) -i;
+		else
+		    is = i;
+		if ((i = egetn(p2)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    i = is < -(tcsh_number_t) -i;
+		else
+		    i = is < (tcsh_number_t) i;
 		break;
 
 	    case LSS | 1:
-		i = egetn(p1) <= egetn(p2);
+		if ((i = egetn(p1)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    is = -(tcsh_number_t) -i;
+		else
+		    is = i;
+		if ((i = egetn(p2)) &
+		    ~(~(unsigned tcsh_number_t) 0 >> 1))
+		    i = is <= -(tcsh_number_t) -i;
+		else
+		    i = is <= (tcsh_number_t) i;
 		break;
 	    }
 	cleanup_until(p1);
sh.exp.c-2.diff (3,612 bytes)   

Issue History

Date Modified Username Field Change
2024-05-02 16:47 MProG10 New Issue
2024-05-02 16:47 MProG10 File Added: sh.decls.h.diff
2024-05-02 16:47 MProG10 File Added: sh.exp.c.diff
2024-05-02 16:47 MProG10 File Added: sh.h.diff
2024-05-02 16:47 MProG10 File Added: sh.set.c.diff
2024-05-03 00:59 MProG10 Note Added: 0004036
2024-05-03 00:59 MProG10 File Added: sh.decls.h-2.diff
2024-05-03 00:59 MProG10 File Added: sh.exp.c-2.diff
2024-05-03 00:59 MProG10 File Added: sh.set.c-2.diff