[bug] ctest argument forwarding is interpreted by the shell and cannot be escaped

[pasbi]

I want to create a conan package and run the unit tests of my library:

[...]
class MyLibRecipe(ConanFile):
    [...]
    def build(self):
        cmake = CMake(self)
        cmake.configure()
        cmake.build()
        cmake.ctest(cli_args=['--output-on-failure', '-E', "'(Util\\|Test).*'"])
    [...]

in a Windows Powershell. Until all tests work, I wanted to disable some of them using the -E option of ctest.

Unfortunately, conan create fails:

mylib/0.2.0: RUN: ctest --build-config Release --parallel 12 --output-on-failure -E '(Util|Test).*' The command "Test).'" is either misspelled or could not be found.

I tried some options to escape the |, but somehow Powershell seems to interpret it as the pipe operator and wants to run the Test command always (i.e., the error message is the same for all following alternatives).

1. "(Util|Test).*"
2. "'(Util|Test).*'"
3. "'(Util\\|Test).*'"

How am I supposed to escape the | character properly? I think this is a bug, the expected behavior is that option (1) works because it's quite uncommon to let the shell implicitly interpret strings passed through some API.

[czoido]

Hi @pasbi,

Thanks for reporting. I think this issue could be caused by how PowerShell interprets single vs double quotes. Changin them in your example works for me, could you please try to use:

cmake.ctest(cli_args=['--output-on-failure', '-E', '"(Util |Test).*"'])

instead of "'(Util|Test).*'"

Hope this helps.

[pasbi]

Swapping the quotes did the trick, thanks!

'"(Util|Test).*"'

Though, I don't understand what's happening. The outer single quotes denote the python string, so the actual argument is just "(Util|Test).*". I'm not very fluent in Powershell, but IIRC, the quotes work similar to bash. So why is '(Util|Test).*' interpreted but "(Util|Test).*" isn't? ls '(foo|bar).*' does the same as ls "(foo|bar).*": it's looking for a file with name (foo|bar).*.

Is this really the expected behavior? Could it be that the cli_args-string is not properly sanitized and guarded, hence the first ' actually ends a string? That'd be a possible security issue (vulnerable to injection attacks).

[memsharded]

There is no "sanitization" at all in the cli_args the code is just:

def ctest(..., cli_args...):
    ...
    arg_list.extend(cli_args or [])
    arg_list = " ".join(filter(None, arg_list))
    command = f"ctest {arg_list}"

If you need better control, you might compose your actual command and run it with self.run("ctest ..."). You might check self.conf.get("tools.env.virtualenv:powershell") to see if the environment is powershell or not.

That'd be a possible security issue (vulnerable to injection attacks).

This is generally not a concern. You are already executing Python scripts in the machine, which can already run any arbitrary code, subprocesses, etc. Either the full recipe is trusted or not, but having an attack on cli_args would be mostly unnecessary if it is already executing arbitrary executables like those tests.

[pasbi]

It's just a potential issue because it's undocumented and I didn't expect this behavior, and I suppose I'm not alone. That's a problem because it's (a) annoying and (b) potentially dangerous because it is more powerful than expected.

I agree the scenario where this could lead to real security issues is rather artificial (imagine some cloud service having a fixed conan pipeline except the user can select the tests they wan to run). We have seen more contrived vulnerabilities being exploited in the wild.

Similar APIs (like e.g. Python's subprocess.run are not interpreted by the shell unless you explicitly request that (shell=True). Please consider to make conan behave similar. I suppose the same arguments hold for other usages of _conanfile.run.

Anyway, I don't think I'm able to make a pull request to address this and I totally understand that there are other, more important things to fix and to improve. I don't want to argue, you're completely right with everything you say in your comment.

My personal problem has been solved (thanks again!), but I think this issue should not be closed because the problem it describes is real.