Security issue: Possible command injection

[fadi-botros]

https://github.com/Moatasem-Elsayed/cpp-manage-git/blob/3922ef1e7f252ed8488d49779e42670a9e3f401a/gitmanager.cpp#L66

std::system in C++, and its equivalent in all other languages, are extremely dangerous functions, because they pass the string given directly to the OS. So, sanitizing (or escaping) the input is necessary for it.

Imagine you are adding something to the command string, and the user input something like (DON'T TRY THIS):

; rm -rf /

Just a simple input like this, and you would be done with ALL your files, VERY DANGEROUS.

So, try to sanitize, or at least escape anything you add to a command string.

Like for example, search for any ; and add a \ before it, search for any \ and put \ before it, etc... i.e. search for any sensitive character that is recognized by sh and add an \ before it

[Moatasem-Elsayed]

yes I see it , it is really a common issue , I will try to fix it and push a new patch

[Moatasem-Elsayed]

@fadi-botros please check the commit 85e1b9a0f89a71a97efce125dc1d9ff636e9b9c3 , I tried to put handle for especial characters

[fadi-botros]

@Moatasem-Elsayed Checked the commit, good solution, but not perfect

It still maybe a valid requirement to add a file containing for example (spacebar).

I think you should try escaping instead of erroring, escaping means adding \ before special characters.

Something like:

class EscapedString {
    private: std::string escapedString;
    public: EscapedString(const std::string &stringToEscape) {
        std::string result;
        // Assume every letter is to be escaped
        result.reserve(stringToEscape.size() * 2);
        ...
    }

    public std::string string() {
        return escapedString;
    }
};

/// Safe wrapper for `std::system`
bool callSystem(const EscapedString &escapedString) {
    return std::system(escapedString.string());
}
// Try to ensure no one calls `std::system` after this line
#pragma GCC poison std::system

Of course this code is just for demonstration purpose, in reality things are more complex, and this code won't work.