CVE-2019-14868: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: In ksh version 20120801, a flaw was found in the way it evaluates certain environment variables. An attacker could use this flaw to override or bypass environment restrictions to execute shell commands. Services and applications that allow remote unauthenticated attackers to provide one of those environment variables could allow them to exploit this issue remotely.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 7.8
Published: 2020-04-02T17:15Z
Repository: https://github.com/att/ast
Language: C
Files changed: CHANGELOG.md, src/cmd/ksh93/sh/arith.c, src/cmd/ksh93/tests/subshell.sh
Commit date: 2019-12-17 20:16:58 -0800
Commit message: Harden env var imports

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 915ee7a6..12bdf766 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,10 +2,12 @@
 
 ## Deprecations
 
 ## Notable non-backward compatible changes
 
+- You can no longer use arbitrary expressions in imported numeric env vars;
+  only integer literals are allowed.
 - The `login` and `newgrp` special builtins have been removed. Use
   `exec login` and `exec newgrp` instead (#1348).
 
 ## Notable fixes and improvements
 
diff --git a/src/cmd/ksh93/sh/arith.c b/src/cmd/ksh93/sh/arith.c
index 30b30675..8e68cbdc 100644
--- a/src/cmd/ksh93/sh/arith.c
+++ b/src/cmd/ksh93/sh/arith.c
@@ -565,22 +565,35 @@ void *sh_arithcomp(Shell_t *shp, char *str) {
 Sfdouble_t sh_strnum(Shell_t *shp, const char *str, char **ptr, int mode) {
     Sfdouble_t d;
     char *last;
 
     if (*str == 0) {
-        if (ptr) *ptr = (char *)str;
-        return 0;
-    }
-    errno = 0;
-    d = number(str, &last, shp->inarith ? 0 : 10, NULL);
-    if (*last) {
-        if (*last != '.' || last[1] != '.') {
-            d = strval(shp, str, &last, arith, mode);
-            Varsubscript = true;
+        d = 0.0;
+        last = (char *)str;
+    } else {
+        d = number(str, &last, shp->inarith ? 0 : 10, NULL);
+        if (*last && !shp->inarith && sh_isstate(shp, SH_INIT)) {
+            // This call is to handle "base#value" literals if we're importing untrusted env vars.
+            d = number(str, &last, 0, NULL);
+        }
+        if (*last) {
+            if (sh_isstate(shp, SH_INIT)) {
+                // Initializing means importing untrusted env vars. Since the string does not appear
+                // to be a recognized numeric literal give up. We can't safely call strval() since
+                // that allows arbitrary expressions which would create a security vulnerability.
+                d = 0.0;
+            } else {
+                if (*last != '.' || last[1] != '.') {
+                    d = strval(shp, str, &last, arith, mode);
+                    Varsubscript = true;
+                }
+                if (!ptr && *last && mode > 0) {
+                    errormsg(SH_DICT, ERROR_exit(1), e_lexbadchar, *last, str);
+                }
+            }
+        } else if (d == 0.0 && *str == '-') {
+            d = -0.0;
         }
-        if (!ptr && *last && mode > 0) errormsg(SH_DICT, ERROR_exit(1), e_lexbadchar, *last, str);
-    } else if (!d && *str == '-') {
-        d = -0.0;
     }
     if (ptr) *ptr = last;
     return d;
 }
diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh
index b63a8051..3faba475 100644
--- a/src/cmd/ksh93/tests/subshell.sh
+++ b/src/cmd/ksh93/tests/subshell.sh
@@ -854,5 +854,28 @@ foo=$(print `echo bar`)
 # Buffer boundary tests
 for exp in 65535 65536
 do    got=$($SHELL -c 'x=$(printf "%.*c" '$exp' x); print ${#x}' 2>&1)
     [[ $got == $exp ]] || log_error "large command substitution failed" "$exp" "$got"
 done
+
+# ==========
+# Verify that importing untrusted env vars does not allow evaluating arbitrary expressions but does
+# recognize all integer literals recognized by ksh.
+expect=8
+actual=$(env SHLVL='7' $SHELL -c 'echo $SHLVL')
+[[ $actual == $expect ]] || log_error "decimal int literal not recognized" "$expect" "$actual"
+
+expect=14
+actual=$(env SHLVL='013' $SHELL -c 'echo $SHLVL')
+[[ $actual == $expect ]] || log_error "leading zeros int literal not recognized" "$expect" "$actual"
+
+expect=4
+actual=$(env SHLVL='2#11' $SHELL -c 'echo $SHLVL')
+[[ $actual == $expect ]] || log_error "base#value int literal not recognized" "$expect" "$actual"
+
+expect=12
+actual=$(env SHLVL='16#B' $SHELL -c 'echo $SHLVL')
+[[ $actual == $expect ]] || log_error "base#value int literal not recognized" "$expect" "$actual"
+
+expect=1
+actual=$(env SHLVL="2#11+x[\$($bin_echo DANGER WILL ROBINSON >&2)0]" $SHELL -c 'echo $SHLVL')
+[[ $actual == $expect ]] || log_error "expression allowed on env var import" "$expect" "$actual"

Vulnerable code:         if (ptr) *ptr = (char *)str;
        return 0;
    }
    errno = 0;
    d = number(str, &last, shp->inarith ? 0 : 10, NULL);
    if (*last) {
        if (*last != '.' || last[1] != '.') {
            d = strval(shp, str, &last, arith, mode);
            Varsubscript = true;
        if (!ptr && *last && mode > 0) errormsg(SH_DICT, ERROR_exit(1), e_lexbadchar, *last, str);
    } else if (!d && *str == '-') {
        d = -0.0;
Fixed code: - You can no longer use arbitrary expressions in imported numeric env vars;
  only integer literals are allowed.
        d = 0.0;
        last = (char *)str;
    } else {
        d = number(str, &last, shp->inarith ? 0 : 10, NULL);
        if (*last && !shp->inarith && sh_isstate(shp, SH_INIT)) {
            // This call is to handle "base#value" literals if we're importing untrusted env vars.
            d = number(str, &last, 0, NULL);
        }
        if (*last) {
            if (sh_isstate(shp, SH_INIT)) {
                // Initializing means importing untrusted env vars. Since the string does not appear
                // to be a recognized numeric literal give up. We can't safely call strval() since
                // that allows arbitrary expressions which would create a security vulnerability.
                d = 0.0;
            } else {
                if (*last != '.' || last[1] != '.') {
                    d = strval(shp, str, &last, arith, mode);
                    Varsubscript = true;
                }
                if (!ptr && *last && mode > 0) {
                    errormsg(SH_DICT, ERROR_exit(1), e_lexbadchar, *last, str);
                }
            }
        } else if (d == 0.0 && *str == '-') {
            d = -0.0;

# ==========
# Verify that importing untrusted env vars does not allow evaluating arbitrary expressions but does
# recognize all integer literals recognized by ksh.
expect=8
actual=$(env SHLVL='7' $SHELL -c 'echo $SHLVL')
[[ $actual == $expect ]] || log_error "decimal int literal not recognized" "$expect" "$actual"

expect=14
actual=$(env SHLVL='013' $SHELL -c 'echo $SHLVL')
[[ $actual == $expect ]] || log_error "leading zeros int literal not recognized" "$expect" "$actual"

expect=4
actual=$(env SHLVL='2#11' $SHELL -c 'echo $SHLVL')
[[ $actual == $expect ]] || log_error "base#value int literal not recognized" "$expect" "$actual"

expect=12
actual=$(env SHLVL='16#B' $SHELL -c 'echo $SHLVL')
[[ $actual == $expect ]] || log_error "base#value int literal not recognized" "$expect" "$actual"

expect=1
actual=$(env SHLVL="2#11+x[\$($bin_echo DANGER WILL ROBINSON >&2)0]" $SHELL -c 'echo $SHLVL')
[[ $actual == $expect ]] || log_error "expression allowed on env var import" "$expect" "$actual"