#2387 SciTE automatic untrusted code execution

[F8 is great]

Hello.

I would like to raise what I believe to be a security issue regarding a combination of two intentional features of SciTE.

SciTE can trivially be made to execute arbitrary commands when opening files, by setting option "command.discover.properties" (feature 1) in a local SciTE.properties file (feature 2).

This is unsafe in scenarios where a folder contains content that the user did not themselves create, such as:

• The user opens a file in a folder shared with other users.
• The user opens a text file from repositories like SourceForge, GitHub, NPM, or CPAN.
• The user opens a text file from archives downloaded from the internet (even if the content is not related to coding).
• The user opens a text file from co-workers they would normally trust.

To clarify the co-worker example, if the co-worker has used SciTE to open files from GitHub which triggered malicious code, the malicious code could then automatically apply the same attack to other repos on the same machine, or even steal their SSH keys, allowing the attacker to impersonate them.

I have confirmed this behaviour in SciTE 5.3.0 and 5.3.6.

For comparison, VScode has patched similar issues:

• https://blog.ammaraskar.com/vscode-rce/
• https://code.visualstudio.com/blogs/2021/07/06/workspace-trust

I believe git itself has also fixed similar issues (immediate code execution when cloning repos).

Proof of concept

Please see the attached (harmless) proof-of-concept zip file. (Although please note that I accept no liability)

The archive includes a SciTE.properties file that defines a value for "command.discover.properties".
In this instance, the value supplied is a windows command-line to create a file indicating that the code has been run.

If the zip is extracted into a folder and a user opens "arbitrary.txt" in SciTE, the command executes, creating file "bad.txt"

To make the attack more subtle, file SciTE.properties is given the hidden attribute, so it will not be visible in many file browsers.
(In a practical attack, the file could also be included in a folder with many other files, making it harder to spot)

Fix options

I suspect there are many ways this could be addressed, including:

• Make option "command.discover.properties" impossible to set in a local properties file.
• Disable "command.discover.properties" by default, and add another configuration to enable it under certain circumstances. (This would require that the configuration to enable it could not also be defined in a local SciTE.properties file)
• Disable "command.discover.properties" by default, and implement a "trusted workspace" scheme.
• Remove "command.discover.properties" altogether

Limits of this report

I have not exhaustively searched for similar code execution hooks, so there may be other similar features that might also need to be addressed before SciTE could be considered safe for general usage.

I have also not fully understood if there are implications for projects using Scintilla, or if this issue is purely restricted to SciTE.

Thanks

Thank you very much for your time, and continued efforts in making SciTE such a useful utility.

[Neil Hodgson]

Disable "command.discover.properties" by default, and add another configuration to enable it under certain circumstances. (This would require that the configuration to enable it could not also be defined in a local SciTE.properties file)

This is likely reasonable. Require discover.properties=1 in user properties or above (global properties) before using command.discover.properties.

@@ -593,6 +593,7 @@
    SetFileName(absPath);

    propsDiscovered.Clear();

+   if (propsUser.GetInt("discover.properties")) {
    std::string discoveryScript = props.GetExpandedString("command.discover.properties");
    if (discoveryScript.length()) {
        std::string propertiesText = CommandExecute(GUI::StringFromUTF8(discoveryScript).c_str(),
@@ -601,6 +602,7 @@
            propsDiscovered.ReadFromMemory(propertiesText.c_str(), propertiesText.size(), absPath.Directory(), filter, nullptr, 0);
        }
    }
+   }
    CurrentBuffer()->props = propsDiscovered;
    CurrentBuffer()->overrideExtension = "";
    ReadProperties();

[F8 is great]

Thank you for the quick response.
Confirmed, I can see how this would fix the issue.

I do have some follow-up questions:

• This code appears to be purely limited to SciTE, and does not affect Scintilla. Is that correct?
• If I am reading the project history in Mercurial correctly, this issue was first introduced in commit 7bd54888215d from 2011-08-05, and was first released under v2.29, and affects every version since. Is that correct?
• I could not find any other instances of CommandExecute being used in the codebase. Are there any other command execution hooks using a different mechanism that might warrant investigation?
• Are you now intending to implement your suggested fix?
• After a fixed build is made publicly available, do you think it would make sense to make this issue publicly visible, for transparency? (If possible)
• Are you amenable to requesting a CVE number for this issue?

[Neil Hodgson]

• Command execution is only performed by SciTE, not Scintilla or Lexilla.
• command.discover.properties was introduced 2011-08-05 and released in 2.29
• There are many command execution techniques. The main one is the JobQueue subsystem with potential calls to CreateProcess, ShellExecuteEx, and help display with WinHelpW and HHCTRL.OCX. For GTK, the g_spawn_async and g_spawn_async_with_pipes calls are used. There is also the Lua scripting feature with Lua allowing os.execute, io.popen and possibly other calls. SciTE may be controlled by a director application and may ask that application to perform commands through IPC.
• Its likely the proposed change will be committed in the near future.
• I have made this issue public.
• My experience with CVE systems is that CVEs are often unclear or incorrect and they get distributed to many downstream repositories. There is no clear way to update all the downstreams when changes are made so they are full of out-of-date information. I have no interest in participating in CVEs but others may have other opinions.

[F8 is great]

I would suggest that this issue should remain private for now; making this issue public before a fixed build is released puts users at risk.
(According to the main download page, the latest version still appears to be 5.3.6, which I believe is vulnerable)

[Neil Hodgson]

The degree of risk appears low to me and its better to be completely open with open-source projects.

[Neil Hodgson]

Committed as [0ee29b].