cve-2022-0847: fixed version check

[exploide]

Operator precedence or evaluation order was causing troubles here with the version check. Fixed it via grouping the comparisons that belong together.

While I tested it a bit, please double check if this looks correct.

[diego-treitos]

Hello, thank you for your contribution and sorry for the delay on this.

First of all, you can group the boolean tests with parentheses in POSIX. However in this case I fail to see the need for them, although I might be wrong. My understanding is that if anything inside a group of or (||) is evaulated to true, nothing else will be checked. Because the other operators are and (&&) they need to be evaluated both before checking the next or.

I would need an example (a version number) where this evaluation fails.

[exploide]

Consider for example kernel version 5.4.0, which should not be vulnerable. The following test script only returns this result with the patched comparison:

#!/usr/bin/env bash

kernel="5.4.0"

v1=$(echo "$kernel" | cut -d '.' -f1)
v2=$(echo "$kernel" | cut -d '.' -f2)
v3=$(echo "$kernel" | cut -d '.' -f3)
v1=${v1:-0};v2=${v2:-0};v3=${v3:-0};

if [ "$v1" = 5 ]; then
  if [ $((v2)) -lt 8 ] ||
    [ "$v2" = 10 ] && [ $((v3)) -ge 102 ] ||
    [ "$v2" = 15 ] && [ $((v3)) -ge 25 ] ||
    [ "$v2" = 16 ] && [ $((v3)) -ge 11 ] ||
    [ $((v2)) -gt 16 ]
  then
      # Not vulnerable
      echo "without patch: not vulnerable"
  else
      echo "without patch: vulnerable"
  fi
fi

if [ "$v1" = 5 ]; then
  if [ $((v2)) -lt 8 ] ||
    { [ "$v2" = 10 ] && [ $((v3)) -ge 102 ]; } ||
    { [ "$v2" = 15 ] && [ $((v3)) -ge 25 ]; } ||
    { [ "$v2" = 16 ] && [ $((v3)) -ge 11 ]; } ||
    [ $((v2)) -gt 16 ]
  then
      # Not vulnerable
      echo "with patch: not vulnerable"
  else
      echo "with patch: vulnerable"
  fi
fi

without patch: vulnerable
with patch: not vulnerable

[diego-treitos]

Wow, I am very surprised by this. I've been doing some tests and now I get it.

My idea was that true OR <whatever_you_put_here> would be always true.

That was my mistake. It actually works like this, in the example you posted:

true || false && false || false && false ... everything false
------------
     true     && false || false && false ... everything false
     -----------------
           false       || false && false ... everything false
           --------------------------------------------------
                               FALSE

Thank you for making my realize of my mistake and the effort you put on it. However I still think that using parenthesis is better (less characters and more common syntax):

if [ "$v1" = 5 ]; then
  if [ $((v2)) -lt 8 ] ||
    ( [ "$v2" = 10 ] && [ $((v3)) -ge 102 ] ) ||
    ( [ "$v2" = 15 ] && [ $((v3)) -ge 25 ] ) ||
    ( [ "$v2" = 16 ] && [ $((v3)) -ge 11 ] ) ||
    [ $((v2)) -gt 16 ]
  then
      # Not vulnerable
      echo "without patch: not vulnerable"
  else
      echo "without patch: vulnerable"
  fi
fi

Do you think you can make that change so I can merge your PR? or if you prefer I can implement the change directly.

[exploide]

Do you think you can make that change so I can merge your PR?

Sure, I can do it in a minute. I already have the editor open, however, shellcheck tells me "Use { ..; } instead of (..) to avoid subshell overhead". SC2235

Do you prefer (..) anyway?

[diego-treitos]

Damn, you are 100% right. I will merge your version. Thank you for making me realize that () spawns another shell instance. Hehe, I am learning a lot today :D.