[bug] Remote Code Execution Using eval in Exception Handling

[l1k3beef]

Describe the bug

comfy_mtb/nodes/graph_utils.py

Line 479 in 9651a70

"""Node to evaluate a simple math expression string"""

class MTB_MathExpression:
    """Node to evaluate a simple math expression string"""

    @classmethod
    def INPUT_TYPES(cls):
        return {
            "required": {
                "expression": ("STRING", {"default": "", "multiline": True}),
            }
        }

    FUNCTION = "eval_expression"
    RETURN_TYPES = ("FLOAT", "INT")
    RETURN_NAMES = ("result (float)", "result (int)")
    CATEGORY = "mtb/math"
    DESCRIPTION = (
        "evaluate a simple math expression string (!! Fallsback to eval)"
    )

    def eval_expression(self, expression, **kwargs):
        from ast import literal_eval

        for key, value in kwargs.items():
            print(f"Replacing placeholder <{key}> with value {value}")
            expression = expression.replace(f"<{key}>", str(value))

        result = -1
        try:
            result = literal_eval(expression)
        except SyntaxError as e:
            raise ValueError(
                f"The expression syntax is wrong '{expression}': {e}"
            ) from e

        except ValueError:
            try:
                expression = expression.replace("^", "**")
                result = eval(expression)
            except Exception as e:
                # Handle any other exceptions and provide a meaningful error message
                raise ValueError(
                    f"Error evaluating expression '{expression}': {e}"
                ) from e

        return (result, int(result))

The eval in the code here can directly receive user input, which is a very unrecommended behavior. Using the mtb plug-in in ComfyUI will lead to remote code execution. This is a security vulnerability. Please fix it in time.

Reproduction

1. add mtb extension to ComfyUI
2. By adding a MathExpression node, add python code in Input similar to import os;os.system("rm -rf /")

Expected behavior

No response

Operating System

Windows (Default)

Comfy Mode

Comfy Portable (embed) (Default)

Console output

No response

Additional context

No response

[melMass]

This can be deleted all together.
But no your example won't work eval is not exec

[l1k3beef]

Because the plugin requires the ComfyUI framework, I haven't built a complete environment yet, but the following POC is enough to illustrate the problem.
I don't quite understand why eval is used in the exception handling process. Literal_eval itself is used to ensure the safety of executing Python code. This is a bit superfluous.

def eval_expression(expression, **kwargs):
    from ast import literal_eval

    for key, value in kwargs.items():
        print(f"Replacing placeholder <{key}> with value {value}")
        expression = expression.replace(f"<{key}>", str(value))

    result = -1
    try:
        result = literal_eval(expression)
    except SyntaxError as e:
        raise ValueError(
            f"The expression syntax is wrong '{expression}': {e}"
        ) from e

    except ValueError:
        try:
            expression = expression.replace("^", "**")
            result = eval(expression)
        except Exception as e:
            # Handle any other exceptions and provide a meaningful error message
            raise ValueError(
                f"Error evaluating expression '{expression}': {e}"
            ) from e

    return (result, int(result))

expression = "__import__('os').system('calc')"
print(eval_expression(expression))

[melMass]

I don't quite understand why eval is used in the exception handling process.

As a fallback.
I will just remove this node completely as said and close this issue once done.

There are better alternatives, ComfyScripts has one but it's a virtual node so this comes with other issues but Essentials does what I tried here properly: https://github.com/cubiq/ComfyUI_essentials/blob/cb5c69c5715230ff6cc1402ddbb5a59473e23202/misc.py#L9

[l1k3beef]

Your seriousness in coding is very respectable, and I am very happy to discuss this issue here.😄