unraid/webgui

logger: Wrap `escapeshellarg` around it

Closed this issue · 1 comments

Commifreak commented

Hi,

I just saw that these two functions:

could be easily tricked (correct me if Iam wrong) to execute code which is not meant to be executed...?

I mean this line: https://github.com/limetech/webgui/blob/9f4373ace5bfc3d2bbb9c9d6360cfd17e9c7fb78/plugins/dynamix.plugin.manager/scripts/plugin#L467

If the variable contains some special chars, it could be treated as seperate command, when getting logged. It may be a valid command for the raw execution via run but Maybe one wants to wrap those variables into escapeshellarg?

Either at logger or just on those lines, processing those inputs.

My idea would be:

Index: plugins/dynamix.plugin.manager/scripts/language
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/plugins/dynamix.plugin.manager/scripts/language b/plugins/dynamix.plugin.manager/scripts/language
--- a/plugins/dynamix.plugin.manager/scripts/language	(revision 9f4373ace5bfc3d2bbb9c9d6360cfd17e9c7fb78)
+++ b/plugins/dynamix.plugin.manager/scripts/language	(date 1670176126440)
@@ -151,7 +151,7 @@
 // Deal with logging message.
 //
 function logger($message) {
-  shell_exec("logger $message");
+  shell_exec("logger ".escapeshellarg($message));
 }
 
 // Interpret a language file
Index: plugins/dynamix.plugin.manager/scripts/plugin
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/plugins/dynamix.plugin.manager/scripts/plugin b/plugins/dynamix.plugin.manager/scripts/plugin
--- a/plugins/dynamix.plugin.manager/scripts/plugin	(revision 9f4373ace5bfc3d2bbb9c9d6360cfd17e9c7fb78)
+++ b/plugins/dynamix.plugin.manager/scripts/plugin	(date 1670176025805)
@@ -284,7 +284,7 @@
 // Deal with logging message.
 //
 function logger($message) {
-  shell_exec("logger $message");
+  shell_exec("logger ".escapeshellarg($message));
 }
 
 // Interpret a plugin file

If Iam not right, please correct me.

Commifreak commented

Stupid me...

The actual command never gets logged and the File paths itself only contains a file path.

Nevermind: For debugging purposes, I would include the actual command for the log. I would create a PR for this.