doCommand is considered harmful and the next generation API

General Development
1

Trying to understand what’s the purpose of doCommand. It appears to be a way to run python code in a different context, record a macro and to log executed Python code in the Python console. This is quite an unusual approach.

First of all, eval is evil. It is a major attack vector that predates modern computers. With FreeCAD’s ability to execute arbitrary code and add-on authors that might not be aware of “doCommand” threats, it’s a game over in a corporate environment. E.g. it would be at least an embarrassment if somebody targets CERN using FreeCAD’s vulnerability.

The second major reason is that its nearly impossible to refactor strings, check their syntax with tools, hard to debug, test in isolation, etc. How to be sure of anything if you don’t see the whole picture?

Isn’t there a different, safe, and as much powerful way to do what the doCommand provides without using eval?

Python’s contextlib might be a way to do it.

Alternatively, why not just use a function? doCommand(print, “Hello”) might be as powerful.

If you need source code and isolation, maybe put each command into a dedicated module (“import workbenchCommand; doCommand(workbenchCommand)”).

Anybody else thinking that doCommand is harmful?

2

I’m talking about python’s API:
https://forum.freecadweb.org/viewtopic.php?t=2609

As you can see in the linked thread, they also took the mental effort to discuss a potential security threat. And it puts the effort on reviewers to always be vigilant when reviewing doCommand related code, in particular when concatenation is involved. In some cases, a special technique is used seemingly to avoid unsafe evaluation.

It appears that the intention was to make them macro recordable and printable to the console.
Well, a module approach might be the easiest way to achieve that without sacrificing the ability to lint doCommand’s python code.

So, let’s introduce some safe API:

# command_module.py

def hello(name):
  print(name)
  
if __name__ == "__main__":
  # do some stuff

# my_workbench.py

import command_module
...
freecad.do_command_module(command_module)
freecad.do_command_function(command_module.hello, "John")

There are obvious benefits of such command module:

  • It is much more readable than strings
  • It can be validated at build time
  • It is secure
  • It can be tested in isolation

Or, even more simple idea: just deprecate doCommand and discourage using it. If developers want something to be printed or macro recorded, just let them define it explicitely.

3

If we want the command to be undoable:

# my_workbench.py
import command_module
...
freecad.do_reversible_command_module(command_module)
freecad.do_reversible_command_function(command_module.hello, "John")

or

# my_workbench.py
import command_module
...
freecad.do_command_module(command_module, reversible=True)
freecad.do_command_function(command_module.hello, "John", reversible=True)
4

And the simplest one is this:

do_command_gui(lambda : FreeCADGui.Snapper.setGrid())

def do_command_gui(l):
    import astunparse
    import inspect
    import ast
    source_code = astunparse.unparse(ast.parse(inspect.getsource(l).strip()).body[0].value.args[0].body)
    FreeCADGui.doCommandGui(source_code)

Here we just need to add astunparse as a dependency

5

Studying Draft/Line tool now. There is do Command in it.

6

if doCommand is used the command is prinzed in python console. Thus it is used quite often the gui tools implementation.

At least in FEM most doCommands are used for this.

7

I do not see where is this supposed vulnerability. doCommand is used from GUI code to do something based on user inputs. I’m pretty sure you will find a place where one can input a string into some dialog to make doCommand execute arbitrary code… but then one can enter the (malicious) code straight into py console, no hacking required…

If you see a doCommand executing some content from an FCStd file, that is indeed a security hole. But it is not a reason to abandon doCommand altogether (same reason is then valid to eradicate eval from python altogether)

8

You can print the code to python console without forcing it to be a string. I’ve just shown an example above

9

Security is only one reason. What do you say about inability to refactor stuff or find references? Lack of static analysis, inability to unit test, etc.

Some Python code is even hardcoded into C++ header files, which is just inadequate.

Not even mentioning the fact that those lines are executed in the main module rather than in an isolated scope.

10

Yes, the actual purpose is for macro recording. When it were only about the execution of Python code then inside C++ we could have used Python’s C API and otherwise normal Python. But to record the Python code we need a string representation of this code.

First of all, eval is evil. It is a major attack vector that predates modern computers.

You can read many things about the vulnerability caused by eval or exec. An interesting article is this one: https://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html
But the risks arise when executing arbitrary strings. However, in FreeCAD we create the strings and execute it. However, a risk can be (and we already had it) when the concatenated string is not escaped properly.

E.g. it would be at least an embarrassment if somebody targets CERN using FreeCAD’s vulnerability.

In the first place it would be embarrassing for the CERN itself. As you wrote when discussing the issues with doCommand it was explicitly designed not to put Python code into a FCStd project that could be executed when loading it. That’s why users must fetch 3rd party FreeCAD add-ons over a separate channel and it’s in their responsibility to make sure the code can be trusted.

This is quite an unusual approach.

Then I wonder how other applications handle macro recording. Do you know any Python-based application that supports macro recording from Python and C++ without using exec, eval or its C equivalent PyRun_String?

Alternatively, why not just use a function? doCommand(print, “Hello”) might be as powerful.

Is it? And how would you call this from within C++ code?

Anybody else thinking that doCommand is harmful?

Sure, it can be harmful if not using properly but it is not per se.

If we come to the conclusion we should replace doCommand with something safer we need a way that the recorded Python code is the same as we have today. The recorded code must be easy to understand by the users so that they can adjust it if needed.

Or, even more simple idea: just deprecate doCommand and discourage using it. If developers want something to be printed or macro recorded, just let them define it explicitely.

You mean to just write the macro code to the file but not executing it? Yes, it can be an option but the problem is that it’s not guaranteed that the code really works.

Some Python code is even hardcoded into C++ header files, which is just inadequate.

Where? If you talk about InitScript.h then you should know that this file is generated from FreeCADInit.py.

11

Here, I’m talking primarily about doCommand in Python. You shouldn’t write the code in a string if you’re already in the Python execution context.

Blender? Not strictly macro recording, but something similar. Not sure how it works, but apparently it compiles the string to bytecode, filters it for harmful opcodes, and puts it to a limited local context. They also have quite a restrictive API, which I think is a good thing.

But again I’m focusing more on calling it from Python. You don’t need access to python source code if you control object properties.

Also nice that Blender print operations not in a Python Console, but in a different window area. Much cleaner.

First, let’s agree on API, then think of implementation. API shouldn’t be dictated by the implementation.

Then, there may be 2 arguments: a callable Object and variable-length Tuple. I assume something like (pseudocode)

PyArg_Parse(...,"O!$", callable, args)
PyObject_Call(callable, args, ...)

But again, first, we agree on API. Bare strings is not a good API. It can’t be parsed by IDE; it can’t be refactored; it can’t be tested by linter; it’s hard to read. Not even talking about the readability of concatenated code.

For starters, let’s add a decorator so that “doCommand” is not used directly by Python code. I’ve provided an example above. It uses ast and introduces a new module - astunparse.

Then, after gathering feedback, we can replace it with C implementation.

Yes, that’s what I’m trying to tell. Python code written in strings is not guaranteed to work. Unless you test it. It may work in the context of Add-on but might not work in the context of a macro. The expression that needs to be executed in a Macro context must be tested separately.

Yeap, I don’t get it. Why it’s not just imported as a module.

12

Actually in the past we did execute the code in the Python execution context. The problem, however, always was that this code couldn’t be recorded. Python’s inspect module didn’t help much because its usage is rather limited and it’s not able to get the source code of arbitrarily nested classes.

For starters, let’s add a decorator so that “doCommand” is not used directly by Python code. I’ve provided an example above. It uses ast and introduces a new module - astunparse.

I tried your example but I always got an exception raised from within the inspect module.

Traceback (most recent call last):
File “”, line 1, in
File “”, line 4, in do_command_gui
File “/usr/lib/python3.6/inspect.py”, line 973, in getsource
lines, lnum = getsourcelines(object)
File “/usr/lib/python3.6/inspect.py”, line 955, in getsourcelines
lines, lnum = findsource(object)
File “/usr/lib/python3.6/inspect.py”, line 786, in findsource
raise OSError(‘could not get source code’)
OSError: could not get source code

13

Are you running from console or from a *.py file? inspect doesn’t work in the interactive mode as there’s no file.
https://stackoverflow.com/questions/57035364/inspect-getsourcelinesobject-shows-oserror-if-run-from-python-shell-but-gives

14

I know a program recording python code for scripting. It is GOM Inspect professional. https://www.gom.com

Johnny

15

You’re right! I tried it from the Python console.