fix(data): sandbox feature-expression parser to prevent code execution (CWE-94)

qlib/data/cache.py

@@ -25,7 +25,6 @@
     hash_args,
     get_redis_connection,
     read_bin,
-    parse_field,
     remove_fields_space,
     normalize_cache_fields,
     normalize_cache_instruments,
@@ -34,6 +33,7 @@
 
 from ..log import get_module_logger
 from .base import Feature
+from .expression_parser import parse_expression
 from .ops import Operators  # pylint: disable=W0611  # noqa: F401
 
 
@@ -540,7 +540,7 @@ def _expression(self, instrument, field, start_time=None, end_time=None, freq="d
             field = remove_fields_space(field)
             # cache unavailable, generate the cache
             _instrument_dir.mkdir(parents=True, exist_ok=True)
-            if not isinstance(eval(parse_field(field)), Feature):
+            if not isinstance(parse_expression(field), Feature):
                 # When the expression is not a raw feature
                 # generate expression cache if the feature is not a Feature
                 # instance

qlib/data/data.py

@@ -28,7 +28,6 @@
     init_instance_by_config,
     register_wrapper,
     get_module_by_module_path,
-    parse_field,
     hash_args,
     normalize_cache_fields,
     code_to_fname,
@@ -38,6 +37,7 @@
 )
 from ..utils.paral import ParallelExt
 from .ops import Operators  # pylint: disable=W0611  # noqa: F401
+from .expression_parser import parse_expression
 
 
 class ProviderBackendMixin:
@@ -394,7 +394,7 @@ def get_expression_instance(self, field):
             if field in self.expression_instance_cache:
                 expression = self.expression_instance_cache[field]
             else:
-                expression = eval(parse_field(field))
+                expression = parse_expression(field)
                 self.expression_instance_cache[field] = expression
         except NameError as e:
             get_module_logger("data").exception(

qlib/data/expression_parser.py

@@ -0,0 +1,136 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+"""Safe parser for qlib feature expression strings.
+
+The qlib feature DSL lets users write expressions such as::
+
+    Ref($close, -2) / Ref($close, -1) - 1
+
+which are transformed by :func:`qlib.utils.parse_field` into::
+
+    Operators.Ref(Feature("close"), -2) / Operators.Ref(Feature("close"), -1) - 1
+
+Historically qlib evaluated the transformed string with the builtin
+``eval`` which allowed arbitrary Python — an attacker who could influence
+a field string (for example through a workflow YAML config) could gain
+code execution (CWE-94).
+
+This module implements a whitelist-based AST evaluator that only permits
+the constructs actually used by the DSL: numeric/string/boolean/None
+literals, tuples/lists, unary +/-, arithmetic and comparison operators,
+and calls to a small set of allowed names (``Feature``, ``PFeature`` and
+``Operators.<op>``).  Anything else — attribute traversal, arbitrary
+name lookups, ``__import__``, generator/lambda/subscript-escape tricks —
+raises :class:`ValueError`.
+"""
+
+from __future__ import annotations
+
+import ast
+from typing import Any, Dict
+
+from ..utils import parse_field
+from .base import Feature, PFeature
+from .ops import Operators
+
+
+# AST nodes that are structurally safe (contain no executable side effects
+# on their own; their children are validated recursively).
+_ALLOWED_NODES = (
+    ast.Expression,
+    ast.Constant,
+    ast.Num,          # legacy alias (<3.8)
+    ast.Str,          # legacy alias (<3.8)
+    ast.NameConstant, # legacy alias (<3.8)
+    ast.UnaryOp,
+    ast.UAdd,
+    ast.USub,
+    ast.BinOp,
+    ast.Add,
+    ast.Sub,
+    ast.Mult,
+    ast.Div,
+    ast.FloorDiv,
+    ast.Mod,
+    ast.Pow,
+    ast.BoolOp,
+    ast.And,
+    ast.Or,
+    ast.Compare,
+    ast.Eq,
+    ast.NotEq,
+    ast.Lt,
+    ast.LtE,
+    ast.Gt,
+    ast.GtE,
+    ast.Tuple,
+    ast.List,
+    ast.Load,
+    ast.Call,
+    ast.keyword,
+    ast.Name,
+    ast.Attribute,
+)
+
+# Only these bare names may appear in an expression.
+_ALLOWED_NAMES = frozenset({"Feature", "PFeature", "Operators", "True", "False", "None"})
+
+
+def _validate(node: ast.AST) -> None:
+    for child in ast.walk(node):
+        if not isinstance(child, _ALLOWED_NODES):
+            raise ValueError(
+                f"Disallowed expression element: {type(child).__name__}"
+            )
+        if isinstance(child, ast.Attribute):
+            # Only ``Operators.<name>`` is allowed; the value must be the
+            # bare name ``Operators``.
+            if not (isinstance(child.value, ast.Name) and child.value.id == "Operators"):
+                raise ValueError("Only attribute access on 'Operators' is allowed")
+            if child.attr.startswith("_"):
+                raise ValueError("Private attribute access is not allowed")
+        if isinstance(child, ast.Name) and child.id not in _ALLOWED_NAMES:
+            raise ValueError(f"Unknown name in expression: {child.id!r}")
+        if isinstance(child, ast.Call):
+            # keyword arguments are OK, but reject **kwargs / *args unpacking
+            # that could smuggle arbitrary objects.
+            for kw in child.keywords:
+                if kw.arg is None:
+                    raise ValueError("**kwargs is not allowed in expressions")
+
+
+def _safe_namespace() -> Dict[str, Any]:
+    return {
+        "__builtins__": {},
+        "Feature": Feature,
+        "PFeature": PFeature,
+        "Operators": Operators,
+        "True": True,
+        "False": False,
+        "None": None,
+    }
+
+
+def parse_expression(field: str) -> Any:
+    """Parse a qlib feature-DSL string and return the expression object.
+
+    This is the safe replacement for ``eval(parse_field(field))``.
+
+    Parameters
+    ----------
+    field:
+        The user-facing feature expression, e.g. ``"Ref($close, -1)"``.
+
+    Raises
+    ------
+    ValueError
+        If ``field`` contains anything outside of the feature DSL.
+    SyntaxError
+        If the transformed expression is not valid Python.
+    """
+    transformed = parse_field(field)
+    tree = ast.parse(transformed, mode="eval")
+    _validate(tree)
+    code = compile(tree, filename="<qlib-expression>", mode="eval")
+    # pylint: disable=eval-used  # sandboxed: AST whitelist + no builtins
+    return eval(code, _safe_namespace())  # noqa: S307

tests/misc/test_expression_parser_safety.py

@@ -0,0 +1,84 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT License.
+"""Regression test for CWE-94: user-controlled feature expressions must not
+be evaluated as arbitrary Python.
+
+The test targets the safe validator directly so it can run without
+optional runtime dependencies (redis, joblib, ...).  It reimplements the
+whitelist portion of ``qlib.data.expression_parser`` and asserts that
+malicious payloads are rejected before any evaluation happens.
+"""
+import ast
+import os
+import unittest
+
+from qlib.utils import parse_field
+
+
+# Mirror qlib.data.expression_parser._ALLOWED_NODES / _ALLOWED_NAMES.
+_ALLOWED_NODES = (
+    ast.Expression, ast.Constant, ast.UnaryOp, ast.UAdd, ast.USub,
+    ast.BinOp, ast.Add, ast.Sub, ast.Mult, ast.Div, ast.FloorDiv, ast.Mod, ast.Pow,
+    ast.BoolOp, ast.And, ast.Or,
+    ast.Compare, ast.Eq, ast.NotEq, ast.Lt, ast.LtE, ast.Gt, ast.GtE,
+    ast.Tuple, ast.List, ast.Load, ast.Call, ast.keyword, ast.Name, ast.Attribute,
+)
+_ALLOWED_NAMES = frozenset({"Feature", "PFeature", "Operators", "True", "False", "None"})
+
+
+def _validate(tree):
+    for c in ast.walk(tree):
+        if not isinstance(c, _ALLOWED_NODES):
+            raise ValueError(type(c).__name__)
+        if isinstance(c, ast.Attribute):
+            if not (isinstance(c.value, ast.Name) and c.value.id == "Operators"):
+                raise ValueError("attr")
+            if c.attr.startswith("_"):
+                raise ValueError("private")
+        if isinstance(c, ast.Name) and c.id not in _ALLOWED_NAMES:
+            raise ValueError("name " + c.id)
+
+
+def _check(field):
+    transformed = parse_field(field)
+    tree = ast.parse(transformed, mode="eval")
+    _validate(tree)
+
+
+class ExpressionParserSafetyTest(unittest.TestCase):
+    def test_benign_expressions_are_accepted(self):
+        for expr in [
+            "$close",
+            "$$my_feat",
+            "Ref($close, -2)/Ref($close, -1) - 1",
+            "Cut($close, 486, None)",
+            "(2*$close-$high-$low)/($high-$low+1e-12)",
+            "If(Gt($close, 1.0), $high, $low)",
+        ]:
+            _check(expr)  # must not raise
+
+    def test_import_payload_is_blocked(self):
+        marker = "/tmp/qlib_cwe94_marker"
+        if os.path.exists(marker):
+            os.unlink(marker)
+        with self.assertRaises((ValueError, SyntaxError)):
+            _check(f"__import__('os').system('touch {marker}')")
+        self.assertFalse(os.path.exists(marker))
+
+    def test_dunder_escape_is_blocked(self):
+        with self.assertRaises((ValueError, SyntaxError)):
+            _check("().__class__.__mro__[-1].__subclasses__()")
+
+    def test_unknown_name_is_blocked(self):
+        with self.assertRaises((ValueError, SyntaxError)):
+            _check("open('/etc/passwd').read()")
+
+    def test_lambda_and_comprehensions_blocked(self):
+        with self.assertRaises((ValueError, SyntaxError)):
+            _check("(lambda: 1)()")
+        with self.assertRaises((ValueError, SyntaxError)):
+            _check("[x for x in ()]")
+
+
+if __name__ == "__main__":
+    unittest.main()