[PATCH 0/1] MR9943: msiexec: Fix escaping quoted arguments passed from Shell.

[Yaroslav Osipov (＠mcm)]

Properties with spaces passed to `msiexec` from shells like Bash may result in re-escaping the string. This results in wrong new quote position and so in MSI error `ERROR_INVALID_COMMAND_LINE`. This patch moves the quote back to its legal position.

Step-by-step behavior after fixes in commit `14b718b69bb8d` (linked with Wine bug https://bugs.winehq.org/show_bug.cgi?id=57024):

1. Shell input:

   wine msiexec /i some.msi INSTALLDIR="C:\\Path with spaces\\data"
2. Shell args parser may remove double quote escaping:

   args\[3\] == "some.msi" args\[4\] == "INSTALLDIR=C:\\Path with spaces\\data"
3. `GetCommandLineW()` returns string with quote moved to the start:

   "msiexec ... some.msi "INSTALLDIR=C:\\Path with spaces\\data""
4. Then MSI parser truncates beginning of the string up to character '=', leading to unbalanced quotes in result value:

   C:\\Path with spaces\\data"
5. MSI fails with error `ERROR_INVALID_COMMAND_LINE` (1639).

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57024

Tests in bug 57024 are fine to reproduce the behavior, but note that `system()` and `execvp()` function passes their input on without extra parsing (without item 2 in the fail trace algorithm above). An example of such test:

```
execlp("msiexec", "msiexec", "/i", msi_path, "/qn", "\"MY_PROPERTY=foo bar\"", NULL);
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9943

[PATCH 1/1] msiexec: Fix escaping quoted arguments passed from Shell.

[Yaroslav Osipov]

From: Yaroslav Osipov <[e-mail]>

Properties with spaces passed to `msiexec` from shells like Bash may result in re-escaping the string. This results in wrong new quote position and so in MSI error `ERROR_INVALID_COMMAND_LINE`. This patch moves the quote back to its legal position.

Step-by-step behavior after fixes in commit `14b718b69bb8d` (linked with Wine bug https://bugs.winehq.org/show_bug.cgi?id=57024):

1. Shell input:

    wine msiexec /i some.msi INSTALLDIR="C:\\Path with spaces\\data"

2. Shell args parser may remove double quote escaping:

    args[3] == "some.msi"
    args[4] == "INSTALLDIR=C:\\Path with spaces\\data"

3. `GetCommandLineW()` returns string with quote moved to the start:

    "msiexec ... some.msi \"INSTALLDIR=C:\\Path with spaces\\data\""

4. Then MSI parser truncates beginning of the string up to character '=', leading to unbalanced quotes in result value:

    C:\\Path with spaces\\data"

5. MSI fails with error `ERROR_INVALID_COMMAND_LINE` (1639).

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57024

Tests in bug 57024 are fine to reproduce the behavior, but note that `system()` and `execvp()` function passes their input on without extra parsing (without item 2 in the fail trace algorithm above). An example of such test:

    execlp("msiexec", "msiexec", "/i", msi_path, "/qn", "\"MY_PROPERTY=foo bar\"", NULL);
---
 programs/msiexec/msiexec.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/programs/msiexec/msiexec.c b/programs/msiexec/msiexec.c
index b4fe9a7e6e8..a646d16691f 100644
--- a/programs/msiexec/msiexec.c
+++ b/programs/msiexec/msiexec.c
@@ -139,7 +139,7 @@ static LPWSTR build_properties(struct string_list *property_list)
 	struct string_list *list;
 	LPWSTR ret, p, value;
 	DWORD len;
-	BOOL needs_quote;
+	BOOL needs_quote, needs_move_first_quote;
 
 	if(!property_list)
 		return NULL;
@@ -160,13 +160,21 @@ static LPWSTR build_properties(struct string_list *property_list)
 			continue;
 		len = value - list->str;
 		*p++ = ' ';
-		memcpy(p, list->str, len * sizeof(WCHAR));
-		p += len;
+
+		/* In props like " \"INSTALLDIR=path\" ", move first quote after the '=' (with a result
+		   of " INSTALLDIR=\"path\" ") to save the quotes balance when MSI parser cuts off
+		   characters up to first '='. (eterbug #18574) */
+		needs_move_first_quote = (*list->str == '"');
+
+		memcpy(p, list->str + needs_move_first_quote, (len - needs_move_first_quote) * sizeof(WCHAR));
+		p += len - needs_move_first_quote;
 		*p++ = '=';
+		if(needs_move_first_quote)
+			*p++ = '\"';
 
 		/* check if the value contains spaces and maybe quote it */
 		value++;
-		needs_quote = *value != '"' && wcschr(value, ' ');
+		needs_quote = (!needs_move_first_quote && *value != '"' && wcschr(value, ' '));
 		if(needs_quote)
 			*p++ = '"';
 		len = lstrlenW(value);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/9943

[Hans Leidekker (＠hans)]

I doubt that native msiexec handles the command line like this. To avoid bash moving quotes you can pipe the command to cmd:

```
$ echo 'msiexec /i some.msi INSTALLDIR="C:\Path with spaces\data"' | wine cmd
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9943#note_127625