PR: https://github.com/mantisbt/mantisbt/pull/2052

The escapeshellcmd() call that is being removed in the proposed PR, was added as part of 0026091 to fix a security issue, so I'm reluctant to remove it, even though it is unlikely that this could be used as an attack vector since $g_graphviz_path can only be altered via changes in config_inc.php.

Since PHP 7.4, the command executed in proc_open() can be given as an array; according to the documentation In this case the process will be opened directly (without going through a shell) and PHP will take care of any necessary argument escaping, so maybe a better solution would be

        # Start dot process
-       $t_command = escapeshellcmd( $this->tool_path() . ' -T' . $p_format );
+       $t_command = [$this->tool_path() , ' -T' . $p_format];

I do not have a Windows environment to test this on, maybe you can confirm ? I will test on Linux later.

I had the issue a few months ago. After a tip from atrol I just placed it directly on C: and then set

$g_dot_tool = 'C:\Graphviz\bin\dot.exe';
$g_neato_tool = 'C:\Graphviz\bin\dot.exe -Kneato';

In the config. I don't have $g_graphviz_path in my config.
Works perfectly!

Perhaps you could do the same?

@diedie2

$g_dot_tool = 'C:\Graphviz\bin\dot.exe';
$g_neato_tool = 'C:\Graphviz\bin\dot.exe -Kneato';

Since 2.27.0, these 2 configs have been obsoleted, and are no longer used anywhere in the codebase. You can remove them from your config...

I don't have $g_graphviz_path in my config.

The path to the tools is built dynamically from $g_graphviz_path (default value is /usr/bin/), concatenated with the tool name (dot, neato or circo) and .exe is appended if on Windows. I don't understand how proc_open() could actually find and execute a command in C:\Graphviz\bin, it would be interesting to find out what is happening.

it would be interesting to find out what is happening.

Probably:

1. He has no spaces in the tool path.
2. He added Graphviz path to the PATH enviroment variable.
3. He has no check is_executable() in MantisBT, otherwise simple concatenation of tool path and tool name should not work.

@degrad I'm currently still on 2.62.2, but thx but the heads-up this has been changed. The space in in the original path (I had Program Files first to) was the problem for me. Thats why @atrol suggested to place it directly on C: so there are no spaces in the path. I left it there bc our server is dedicated for mantisbt.

OK that explains it. Thanks for the feedback @diedie2.

$t_command = [$this->tool_path() , ' -T' . $p_format];

Unfortunately it doesn't work that way on Windows, plus it requires PHP 7.4.0.

I doubt that the escapeshellcmd() function is needed at all in this case, because there is code above that checks the program path for validity by is_executable().

I have tried many options over many hours and the best one is this, it also does the correct masking of spaces:

$t_command = escapeshellarg( $this->tool_path() ) . ' -T' . $p_format;

I have refreshed PR.

In the PR I also added realpath() for Graphviz tools it efficiently fixes any problems with path including wrong directory separators and missing trailing separator.

Unfortunately it doesn't work that way on Windows

So you're saying that this standard PHP syntax does not work on Windows ? The manual states that command may be passed as array of command parameters. In this case the process will be opened directly (without going through a shell) and PHP will take care of any necessary argument escaping..

I must admit I do not fully understand what the following note implies: On Windows, the argument escaping of the array elements assumes that the command line parsing of the executed command is compatible with the parsing of command line arguments done by the VC runtime.

it requires PHP 7.4.0.

Indeed, but this is the minimum required version to run MantisBT, so no problem here.

So you're saying that this standard PHP syntax does not work on Windows ?

I'm not quite sure but I suspect it does the same thing as escapeshellcmd() but internally so no error returns and result is empty, I also monitored file openings with Process Monitor (by Mark Russinovich) and saw no attempt to open or execute any file with good or bad paths. Maybe it depends on the Apache or PHP configuration.