added validation of expressions to prevent malicious code execution via eval()

At the point when eval is called, all symbols should have already been resolved
to their values. That means we only need to allow for numeric characters along
with arithmetic and bitwise operators, round brackets and whitespace. The
character 'x' and the characters 'abcdef' are also accepted to allow for hex
numbers such as 0x123abc. They are only allowed however in sequences starting
with 0x. If any other character is encountered the expression is deemed invalid.

This change also removes the try/except around the eval. As all symbols have
been resolved, the only thing that can be left is a valid expression. If it
does not evaluate, an exception *should* be raised rather than being silently
ignored (and hoping for an exception down the line).

Author: wnienhaus

esp32_ulp/opcodes.py

@@ -6,7 +6,7 @@
 from uctypes import struct, addressof, LITTLE_ENDIAN, UINT32, BFUINT32, BF_POS, BF_LEN
 
 from .soc import *
-from .util import split_tokens
+from .util import split_tokens, validate_expression
 
 # XXX dirty hack: use a global for the symbol table
 symbols = None
@@ -277,11 +277,9 @@ def eval_arg(arg):
         else:
             parts.append(token)
     parts = "".join(parts)
-    try:
-        parts = str(eval(parts))
-    except:
-        pass
-    return parts
+    if not validate_expression(parts):
+        raise ValueError('Unsupported expression: %s' % parts)
+    return eval(parts)
 
 
 def arg_qualify(arg):

esp32_ulp/util.py

@@ -52,3 +52,28 @@ def file_exists(filename):
     except:
         pass
     return False
+
+
+def validate_expression(param):
+    for token in split_tokens(param):
+        state = 0
+        for c in token:
+            if c not in ' \t+-*/%()<>&|~x0123456789abcdef':
+                return False
+
+            # the following allows hex digits a-f after 0x but not otherwise
+            if state == 0:
+                if c in 'abcdef':
+                    return False
+                if c == '0':
+                    state = 1
+                continue
+
+            if state == 1:
+                state = 2 if c == 'x' else 0
+                continue
+
+            if state == 2:
+                if c not in '0123456789abcdef':
+                    state = 0
+    return True

tests/opcodes.py

@@ -1,6 +1,6 @@
 from uctypes import UINT32, BFUINT32, BF_POS, BF_LEN
 from esp32_ulp.opcodes import make_ins, make_ins_struct_def
-from esp32_ulp.opcodes import get_reg, get_imm, get_cond, arg_qualify, ARG, REG, IMM, SYM, COND
+from esp32_ulp.opcodes import get_reg, get_imm, get_cond, arg_qualify, eval_arg, ARG, REG, IMM, SYM, COND
 from esp32_ulp.assemble import SymbolTable, ABS, REL, TEXT
 import esp32_ulp.opcodes as opcodes
 
@@ -72,9 +72,46 @@ def test_get_cond():
     assert get_cond('Eq') == 'eq'
 
 
+def test_eval_arg():
+    opcodes.symbols = SymbolTable({}, {}, {})
+    opcodes.symbols.set_sym('const', ABS, None, 42)  # constant
+    opcodes.symbols.set_sym('raise', ABS, None, 99)  # constant using a python keyword as name (is allowed)
+
+    assert eval_arg('1+1') == 2
+    assert eval_arg('1+const') == 43
+    assert eval_arg('raise*2/3') == 66
+    assert eval_arg('raise-const') == 57
+    assert eval_arg('(raise-const)*2') == 114
+    assert eval_arg('const    % 5') == 2
+    assert eval_arg('const + 0x19af') == 0x19af + 42
+    assert eval_arg('const & ~2') == 40
+    assert eval_arg('const << 3') == 336
+    assert eval_arg('const >> 1') == 21
+    assert eval_arg('(const|4)&0xf') == 0xe
+
+    assert_raises(ValueError, eval_arg, 'evil()')
+    assert_raises(ValueError, eval_arg, 'def cafe()')
+    assert_raises(ValueError, eval_arg, '1 ^ 2')
+    assert_raises(ValueError, eval_arg, '!100')
+
+    # clean up
+    opcodes.symbols = None
+
+
+def assert_raises(exception, func, *args):
+    try:
+        func(*args)
+    except exception:
+        raised = True
+    else:
+        raised = False
+    assert raised
+
+
 test_make_ins_struct_def()
 test_make_ins()
 test_arg_qualify()
 test_get_reg()
 test_get_imm()
 test_get_cond()
+test_eval_arg()

tests/util.py

@@ -1,5 +1,5 @@
 import os
-from esp32_ulp.util import split_tokens, file_exists
+from esp32_ulp.util import split_tokens, file_exists, validate_expression
 
 tests = []
 
@@ -31,6 +31,32 @@ def test_split_tokens():
     assert split_tokens("#test") == ['#', 'test']
 
 
+@test
+def test_validate_expression():
+    assert validate_expression('') is True
+    assert validate_expression('1') is True
+    assert validate_expression('1+1') is True
+    assert validate_expression('(1+1)') is True
+    assert validate_expression('(1+1)*2') is True
+    assert validate_expression('(1 + 1)') is True
+    assert validate_expression('10 % 2') is True
+    assert validate_expression('0x100 << 2') is True
+    assert validate_expression('0x100 & ~2') is True
+    assert validate_expression('0xabcdef') is True
+    assert validate_expression('0x123def') is True
+    assert validate_expression('2*3+4/5&6|7') is True
+    assert validate_expression('(((((1+1) * 2') is True  # valid characters, even if expression is not valid
+
+    assert validate_expression(':') is False
+    assert validate_expression('_') is False
+    assert validate_expression('=') is False
+    assert validate_expression('.') is False
+    assert validate_expression('!') is False
+    assert validate_expression('123 ^ 4') is False  # operator not supported for now
+    assert validate_expression('evil()') is False
+    assert validate_expression('def cafe()') is False  # valid hex digits, but potentially dangerous code
+
+
 @test
 def test_file_exists():
     testfile = '.testfile'