Subject: Potential Bug/Vuln in test.c

From: Nathan Hays <[email-masked]>
Date: Wed, 21 Aug 2024 17:29:27 +0000

# ENV #

Ubuntu 22.04 LTS 
COREUTILS version 8.32-4.1ubuntu1.2

*Note could be specific to Ubuntu but I believe I confirmed in source*

# Observed Behavior #

Expectation:
`var=''; [ -n $var ]; echo $?` should NOT return `0`
Reality:
`var=''; [ -n $var ]; echo $?` returns `0`

*Note that this behavior does not persist when variable is quoted* 

Test:
~~~bash
testvar=''
[ -n $testvar ]
echo -n unquote exit $?
[ -n "$testvar" ]
echo -n quote exit $?
[ -z $testvar ]
echo -z unquote exit $?
[ -z "$testvar" ]
echo -z quote exit $?
~~~

Result:
~~~bash
+ testvar=
+ '[' -n ']'
+ echo -n unquote exit 0
unquote exit 0+ '[' -n '' ']'
+ echo -n quote exit 1
quote exit 1+ '[' -z ']'
+ echo -z unquote exit 0
-z unquote exit 0
+ '[' -z '' ']'
+ echo -z quote exit 0
-z quote exit 0
~~~

# Relevant Code #

>From `coreutils/src/test.c`:

lines 106-111:
~~~C
static void
unary_advance (void)
{
  advance (true);
  ++pos;
}
~~~

lines 512-514:
~~~C
    case 'n':			/* True if arg has some length. */
      unary_advance ();
      return argv[pos - 1][0] != 0;
~~~

# What it looks to me that it's doing #

Based on my interpretation of the relevant code sections, which could be completely wrong:
 
`'[' -n ']'` moves to `']'` and checks if `']'` has a length of `[0]` and returns `0` if not (so always `0`, always `TRUE`)

This would explain why the behavior is not continued when the var is quoted: 

`'[' -n '' ']'` moves to `''` and checks if `''` has a length of `[0]` and returns (not zero) correctly.

Oddly though, the `-z` argument appears to handle both situations correct despite the code being similar. 

# Word Around # 

When using `-n` with `test` (or `[`), ensure variables are always quoted to avoid a false positive when the variable is unset but unquoted.
Use `-z` exclusively instead.

This is my first time reporting anything like this so apologies in advance if formatting or other ways on how to pass on the information could be improved. 

Nathan Hays | Principal Security Consultant | NCC Group PLC