mscs
mscs copied to clipboard
MinecraftServerControl
Usage of non-local environment variables
Currently, the command-handling logic is not placed inside a function, making the usage of local environment variables impossible. This can cause previously set environment variables to get overwritten.
To mitigate the above issue, environmental variables get cleared before usage or directly overwritten. However, in some cases environment variables get appended, but not checked if they are empty before. This can cause external code to change the environmental variable at a specific time, causing injection of unwanted codes into the options of mscs/msctl. This can have multiple effects ranging from a confusing and bloated output of status to a spam of created folders when using start; however, as far as I have seen, there are no security implications, since variables are always enclosed in double quotes and thus not interpreted.
An example of the above: https://github.com/MinecraftServerControl/mscs/blob/8118d107471d09a73416d45f834d6aae034c6204/msctl#L3053-L3058
An example where it's done correctly (note L2668): https://github.com/MinecraftServerControl/mscs/blob/8118d107471d09a73416d45f834d6aae034c6204/msctl#L2667-L2676
#310 fixes the mitigation, however, rewriting the command handling in a function might still be useful.
Nice catch @xathon. I've looked over your PR, and it looks correct to me. Thanks for noting the issue and sending in a patch to correct it!
I don't have much time to work on this code, but would be happy to review patches if you are interested in moving the command handling into a function.
No worries! This was causing me all sorts of headaches, wondering how in the world I had injected an array into my status command 😄
I don't have much time to work on this code, but would be happy to review patches if you are interested in moving the command handling into a function.
I'll have a look. I did a short attempt earlier, which failed due to being unable to pass the correct variables to the handlers. It might be a bigger rewrite, but if I find some time I'll try my hand at it.