From 08f619d15f8f17e352ec558c6f4cd501d9e69205 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Mon, 25 Jul 2022 09:57:43 -0400 Subject: [PATCH 01/17] add motw to scp and sftp --- contrib/win32/win32compat/misc.c | 35 +++++++++++++++++++++++ contrib/win32/win32compat/misc_internal.h | 3 +- scp.c | 8 ++++++ sftp-client.c | 9 ++++++ sftp-server.c | 9 ++++++ 5 files changed, 63 insertions(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index ccdcd3498db..44300756ad6 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2119,3 +2119,38 @@ strrstr(const char *inStr, const char *pattern) return last; } + +int +add_mark_of_web(const char* filename) +{ + // ZoneId=3 indicates the file comes from the Internet Zone + const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; + const DWORD shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; + int status = 0; + char* filepath = NULL; + DWORD numWritten = 0; + BOOL writeResult; + HANDLE file; + size_t filepath_length = strlen(filename) + strlen(":Zone.Identifier") + 1; + + filepath = malloc(filepath_length); + if (filepath == NULL) { + return -1; + } + // create zone identifer file stream and write the Mark of the Web to it + sprintf_s(filepath, filepath_length, "%s:Zone.Identifier", filename); + file = CreateFile(filepath, GENERIC_WRITE, shareMode, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + if (INVALID_HANDLE_VALUE == file) { + status = -1; + goto cleanup; + } + writeResult = WriteFile(file, zoneIdentifier, (DWORD)strlen(zoneIdentifier), &numWritten, NULL); + CloseHandle(file); + if (!writeResult) { + status = -1; + } +cleanup: + free(filepath); + return status; +} + diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h index a3d63c33d20..73a487540ca 100644 --- a/contrib/win32/win32compat/misc_internal.h +++ b/contrib/win32/win32compat/misc_internal.h @@ -82,4 +82,5 @@ wchar_t* get_final_path_by_handle(HANDLE h); int lookup_principal_name(const wchar_t * sam_account_name, wchar_t * user_principal_name); BOOL is_bash_test_env(); int bash_to_win_path(const char *in, char *out, const size_t out_len); -void debug_assert_internal(); \ No newline at end of file +void debug_assert_internal(); +int add_mark_of_web(const char* filename); diff --git a/scp.c b/scp.c index 8bc8dc91b84..d86a486eb80 100644 --- a/scp.c +++ b/scp.c @@ -135,6 +135,9 @@ #include "sftp-common.h" #include "sftp-client.h" +#ifdef WINDOWS +#include "misc_internal.h" +#endif // WINDOWS extern char *__progname; @@ -2074,6 +2077,11 @@ sink(int argc, char **argv, const char *src) omode = mode; mode |= S_IWUSR; #ifdef WINDOWS + if (add_mark_of_web(np) == -1 && verbose_mode) { + run_err("scp: %s: failed to add mark of the web\n", np); + exit(1); + }; + // In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX. if ((ofd = open(np, O_WRONLY|O_CREAT, USHRT_MAX)) == -1) { #else diff --git a/sftp-client.c b/sftp-client.c index d622c842685..fefa35d31f6 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1727,6 +1727,15 @@ do_download(struct sftp_conn *conn, const char *remote_path, } } close(local_fd); +#ifdef WINDOWS + if (add_mark_of_web(local_path) == -1) { + // (resume_flag ? 0 : O_TRUNC) on line 1355 requires us + // to add the mark of the web after transferring the file. + // if MOTW fails, we need to remove the file before exiting + remove(local_path); + fatal_f("%s: failed to add mark of the web", local_path); + } +#endif // WINDOWS sshbuf_free(msg); free(handle); diff --git a/sftp-server.c b/sftp-server.c index 4c012b45765..90759d864b6 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -54,6 +54,9 @@ #include "sftp.h" #include "sftp-common.h" +#ifdef WINDOWS +#include "misc_internal.h" +#endif // WINDOWS char *sftp_realpath(const char *, char *); /* sftp-realpath.c */ @@ -863,6 +866,12 @@ process_write(u_int32_t id) (r = sshbuf_get_string(iqueue, &data, &len)) != 0) fatal_fr(r, "parse"); +#ifdef WINDOWS + if (add_mark_of_web(resolved_path_utf8(handle_to_name(handle))) == -1) { + fatal_f("%s: failed to add mark of the web", resolved_path_utf8(handle_to_name(handle))); + } +#endif // WINDOWS + debug("request %u: write \"%s\" (handle %d) off %llu len %zu", id, handle_to_name(handle), handle, (unsigned long long)off, len); fd = handle_to_fd(handle); From 6b053c8fe9067c16c5b1e82a5860dfa92b430135 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Mon, 25 Jul 2022 13:26:11 -0400 Subject: [PATCH 02/17] retrigger appveyor --- contrib/win32/win32compat/misc.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 44300756ad6..5d8833dcee5 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2127,19 +2127,19 @@ add_mark_of_web(const char* filename) const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; const DWORD shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; int status = 0; - char* filepath = NULL; + char* filestreamPath = NULL; DWORD numWritten = 0; BOOL writeResult; HANDLE file; - size_t filepath_length = strlen(filename) + strlen(":Zone.Identifier") + 1; + size_t filestreampath_length = strlen(filename) + strlen(":Zone.Identifier") + 1; - filepath = malloc(filepath_length); - if (filepath == NULL) { + filestreamPath = malloc(filestreampath_length); + if (filestreamPath == NULL) { return -1; } // create zone identifer file stream and write the Mark of the Web to it - sprintf_s(filepath, filepath_length, "%s:Zone.Identifier", filename); - file = CreateFile(filepath, GENERIC_WRITE, shareMode, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + sprintf_s(filestreamPath, filestreampath_length, "%s:Zone.Identifier", filename); + file = CreateFile(filestreamPath, GENERIC_WRITE, shareMode, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); if (INVALID_HANDLE_VALUE == file) { status = -1; goto cleanup; @@ -2150,7 +2150,7 @@ add_mark_of_web(const char* filename) status = -1; } cleanup: - free(filepath); + free(filestreamPath); return status; } From fefb6acb8c942c3861a305db95cdb1a35834c715 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 26 Jul 2022 13:57:26 -0400 Subject: [PATCH 03/17] fix motw for filepaths with unicode characters --- contrib/win32/win32compat/misc.c | 30 +++++++++++------------ contrib/win32/win32compat/misc_internal.h | 2 +- scp.c | 8 ++++-- sftp-client.c | 6 ++++- sftp-server.c | 6 +++-- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 5d8833dcee5..220de425686 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2121,36 +2121,36 @@ strrstr(const char *inStr, const char *pattern) } int -add_mark_of_web(const char* filename) +add_mark_of_web(const wchar_t* filename) { // ZoneId=3 indicates the file comes from the Internet Zone - const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; + const wchar_t zoneIdentifier[] = L"[ZoneTransfer]\nZoneId=3"; const DWORD shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; - int status = 0; - char* filestreamPath = NULL; + int status = -1; + wchar_t* fileStreamPath = NULL; DWORD numWritten = 0; BOOL writeResult; HANDLE file; - size_t filestreampath_length = strlen(filename) + strlen(":Zone.Identifier") + 1; + size_t fileStreamPathLen = wcslen(filename) + wcslen(L":Zone.Identifier") + 2; - filestreamPath = malloc(filestreampath_length); - if (filestreamPath == NULL) { - return -1; + fileStreamPath = malloc(fileStreamPathLen * sizeof(wchar_t)); + if (fileStreamPath == NULL) { + goto out; } // create zone identifer file stream and write the Mark of the Web to it - sprintf_s(filestreamPath, filestreampath_length, "%s:Zone.Identifier", filename); - file = CreateFile(filestreamPath, GENERIC_WRITE, shareMode, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + swprintf_s(fileStreamPath, fileStreamPathLen, L"%s:Zone.Identifier", filename); + file = CreateFileW(fileStreamPath, GENERIC_WRITE, shareMode, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); if (INVALID_HANDLE_VALUE == file) { - status = -1; goto cleanup; } - writeResult = WriteFile(file, zoneIdentifier, (DWORD)strlen(zoneIdentifier), &numWritten, NULL); + writeResult = WriteFile(file, zoneIdentifier, (DWORD)(wcslen(zoneIdentifier)*sizeof(wchar_t)), &numWritten, NULL); CloseHandle(file); - if (!writeResult) { - status = -1; + if (writeResult) { + status = 0; } cleanup: - free(filestreamPath); + free(fileStreamPath); +out: return status; } diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h index 73a487540ca..1d0d2e880f7 100644 --- a/contrib/win32/win32compat/misc_internal.h +++ b/contrib/win32/win32compat/misc_internal.h @@ -83,4 +83,4 @@ int lookup_principal_name(const wchar_t * sam_account_name, wchar_t * user_princ BOOL is_bash_test_env(); int bash_to_win_path(const char *in, char *out, const size_t out_len); void debug_assert_internal(); -int add_mark_of_web(const char* filename); +int add_mark_of_web(const wchar_t* filename); diff --git a/scp.c b/scp.c index d86a486eb80..0a68db02f10 100644 --- a/scp.c +++ b/scp.c @@ -2077,10 +2077,14 @@ sink(int argc, char **argv, const char *src) omode = mode; mode |= S_IWUSR; #ifdef WINDOWS - if (add_mark_of_web(np) == -1 && verbose_mode) { + wchar_t* filepath = utf8_to_utf16(np); + if (filepath == NULL || add_mark_of_web(filepath) == -1) { + if (filepath) + free(filepath); run_err("scp: %s: failed to add mark of the web\n", np); exit(1); - }; + } + free(filepath); // In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX. if ((ofd = open(np, O_WRONLY|O_CREAT, USHRT_MAX)) == -1) { diff --git a/sftp-client.c b/sftp-client.c index fefa35d31f6..738fc08acd7 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1728,13 +1728,17 @@ do_download(struct sftp_conn *conn, const char *remote_path, } close(local_fd); #ifdef WINDOWS - if (add_mark_of_web(local_path) == -1) { + wchar_t* filepath = utf8_to_utf16(local_path); + if (filepath == NULL || add_mark_of_web(filepath) == -1) { // (resume_flag ? 0 : O_TRUNC) on line 1355 requires us // to add the mark of the web after transferring the file. // if MOTW fails, we need to remove the file before exiting remove(local_path); + if (filepath) + free(filepath); fatal_f("%s: failed to add mark of the web", local_path); } + free(filepath); #endif // WINDOWS sshbuf_free(msg); free(handle); diff --git a/sftp-server.c b/sftp-server.c index 90759d864b6..773f60bc48d 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -867,9 +867,11 @@ process_write(u_int32_t id) fatal_fr(r, "parse"); #ifdef WINDOWS - if (add_mark_of_web(resolved_path_utf8(handle_to_name(handle))) == -1) { - fatal_f("%s: failed to add mark of the web", resolved_path_utf8(handle_to_name(handle))); + wchar_t* filepath = resolved_path_utf16(handle_to_name(handle)); + if (filepath == NULL || add_mark_of_web(filepath) == -1) { + fatal_f("%s: failed to add mark of the web", filepath); } + free(filepath); #endif // WINDOWS debug("request %u: write \"%s\" (handle %d) off %llu len %zu", From 8ae735c87529db1d9837ceddc9750421f8c851ff Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Wed, 27 Jul 2022 14:11:52 -0400 Subject: [PATCH 04/17] modify error handling --- contrib/win32/win32compat/misc.c | 3 +-- scp.c | 11 +++++++---- sftp-client.c | 9 ++++++--- sftp-server.c | 5 ++++- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 220de425686..73f6feaaf37 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2135,7 +2135,7 @@ add_mark_of_web(const wchar_t* filename) fileStreamPath = malloc(fileStreamPathLen * sizeof(wchar_t)); if (fileStreamPath == NULL) { - goto out; + return status; } // create zone identifer file stream and write the Mark of the Web to it swprintf_s(fileStreamPath, fileStreamPathLen, L"%s:Zone.Identifier", filename); @@ -2150,7 +2150,6 @@ add_mark_of_web(const wchar_t* filename) } cleanup: free(fileStreamPath); -out: return status; } diff --git a/scp.c b/scp.c index 0a68db02f10..3e728ccc6f0 100644 --- a/scp.c +++ b/scp.c @@ -2078,10 +2078,13 @@ sink(int argc, char **argv, const char *src) mode |= S_IWUSR; #ifdef WINDOWS wchar_t* filepath = utf8_to_utf16(np); - if (filepath == NULL || add_mark_of_web(filepath) == -1) { - if (filepath) - free(filepath); - run_err("scp: %s: failed to add mark of the web\n", np); + if (filepath == NULL) { + run_err("cannot convert %s to utf16 for mark of the web\n", np); + exit(1); + } + if (add_mark_of_web(filepath) == -1) { + free(filepath); + run_err("%s: failed to add mark of the web\n", np); exit(1); } free(filepath); diff --git a/sftp-client.c b/sftp-client.c index 738fc08acd7..1a602454b3b 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1729,13 +1729,16 @@ do_download(struct sftp_conn *conn, const char *remote_path, close(local_fd); #ifdef WINDOWS wchar_t* filepath = utf8_to_utf16(local_path); - if (filepath == NULL || add_mark_of_web(filepath) == -1) { + if (filepath == NULL) { // (resume_flag ? 0 : O_TRUNC) on line 1355 requires us // to add the mark of the web after transferring the file. // if MOTW fails, we need to remove the file before exiting remove(local_path); - if (filepath) - free(filepath); + fatal_f("%s: cannot convert %s to utf16 for mark of the web", local_path); + } + if (add_mark_of_web(filepath) == -1) { + remove(local_path); + free(filepath); fatal_f("%s: failed to add mark of the web", local_path); } free(filepath); diff --git a/sftp-server.c b/sftp-server.c index 773f60bc48d..67b6d50edcc 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -868,7 +868,10 @@ process_write(u_int32_t id) #ifdef WINDOWS wchar_t* filepath = resolved_path_utf16(handle_to_name(handle)); - if (filepath == NULL || add_mark_of_web(filepath) == -1) { + if (filepath == NULL) { + fatal_f("%s: cannot convert %s to utf16 for mark of the web", filepath); + } + if (add_mark_of_web(filepath) == -1) { fatal_f("%s: failed to add mark of the web", filepath); } free(filepath); From a0e36bc3422b28e32c7ee0d436ddb1e68196f893 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 28 Jul 2022 16:42:20 -0400 Subject: [PATCH 05/17] add debug for appveyor --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index 990da0d723b..fa31d82683b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -18,6 +18,7 @@ after_build: test_script: - ps: | + $blockRdp = $true; iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1')) Import-Module $env:APPVEYOR_BUILD_FOLDER\contrib\win32\openssh\AppveyorHelper.psm1 Invoke-OpenSSHTests From 3fe5bbaf63cd166e9eccdf25f0aa9345bb98bef3 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 4 Aug 2022 10:44:59 -0400 Subject: [PATCH 06/17] modify motw method to use openssh method to open filestream --- appveyor.yml | 1 - contrib/win32/win32compat/misc.c | 34 +++++++++++------------ contrib/win32/win32compat/misc_internal.h | 2 +- scp.c | 9 +----- sftp-client.c | 9 +----- sftp-server.c | 4 +-- 6 files changed, 21 insertions(+), 38 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index fa31d82683b..990da0d723b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -18,7 +18,6 @@ after_build: test_script: - ps: | - $blockRdp = $true; iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1')) Import-Module $env:APPVEYOR_BUILD_FOLDER\contrib\win32\openssh\AppveyorHelper.psm1 Invoke-OpenSSHTests diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 73f6feaaf37..b71847bd1dd 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -60,6 +60,7 @@ #include "w32fd.h" #include "inc\string.h" #include "inc\time.h" +#include "..\..\..\atomicio.h" #include @@ -2121,32 +2122,29 @@ strrstr(const char *inStr, const char *pattern) } int -add_mark_of_web(const wchar_t* filename) +add_mark_of_web(const char* filename) { // ZoneId=3 indicates the file comes from the Internet Zone - const wchar_t zoneIdentifier[] = L"[ZoneTransfer]\nZoneId=3"; - const DWORD shareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; - int status = -1; - wchar_t* fileStreamPath = NULL; - DWORD numWritten = 0; - BOOL writeResult; - HANDLE file; - size_t fileStreamPathLen = wcslen(filename) + wcslen(L":Zone.Identifier") + 2; - - fileStreamPath = malloc(fileStreamPathLen * sizeof(wchar_t)); + const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; + char* fileStreamPath = NULL; + size_t fileStreamPathLen = strlen(filename) + strlen(":Zone.Identifier") + 1; + int ofd, status = 0; + + fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); if (fileStreamPath == NULL) { return status; } // create zone identifer file stream and write the Mark of the Web to it - swprintf_s(fileStreamPath, fileStreamPathLen, L"%s:Zone.Identifier", filename); - file = CreateFileW(fileStreamPath, GENERIC_WRITE, shareMode, NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); - if (INVALID_HANDLE_VALUE == file) { + sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); + if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { + status = -1; goto cleanup; } - writeResult = WriteFile(file, zoneIdentifier, (DWORD)(wcslen(zoneIdentifier)*sizeof(wchar_t)), &numWritten, NULL); - CloseHandle(file); - if (writeResult) { - status = 0; + if (atomicio(vwrite, ofd, zoneIdentifier, strlen(zoneIdentifier)) != strlen(zoneIdentifier)) { + status = -1; + } + if (close(ofd) == -1) { + status = -1; } cleanup: free(fileStreamPath); diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h index 1d0d2e880f7..73a487540ca 100644 --- a/contrib/win32/win32compat/misc_internal.h +++ b/contrib/win32/win32compat/misc_internal.h @@ -83,4 +83,4 @@ int lookup_principal_name(const wchar_t * sam_account_name, wchar_t * user_princ BOOL is_bash_test_env(); int bash_to_win_path(const char *in, char *out, const size_t out_len); void debug_assert_internal(); -int add_mark_of_web(const wchar_t* filename); +int add_mark_of_web(const char* filename); diff --git a/scp.c b/scp.c index 3e728ccc6f0..5fbdc279d4a 100644 --- a/scp.c +++ b/scp.c @@ -2077,17 +2077,10 @@ sink(int argc, char **argv, const char *src) omode = mode; mode |= S_IWUSR; #ifdef WINDOWS - wchar_t* filepath = utf8_to_utf16(np); - if (filepath == NULL) { - run_err("cannot convert %s to utf16 for mark of the web\n", np); - exit(1); - } - if (add_mark_of_web(filepath) == -1) { - free(filepath); + if (add_mark_of_web(np) == -1) { run_err("%s: failed to add mark of the web\n", np); exit(1); } - free(filepath); // In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX. if ((ofd = open(np, O_WRONLY|O_CREAT, USHRT_MAX)) == -1) { diff --git a/sftp-client.c b/sftp-client.c index 1a602454b3b..fefa35d31f6 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1728,20 +1728,13 @@ do_download(struct sftp_conn *conn, const char *remote_path, } close(local_fd); #ifdef WINDOWS - wchar_t* filepath = utf8_to_utf16(local_path); - if (filepath == NULL) { + if (add_mark_of_web(local_path) == -1) { // (resume_flag ? 0 : O_TRUNC) on line 1355 requires us // to add the mark of the web after transferring the file. // if MOTW fails, we need to remove the file before exiting remove(local_path); - fatal_f("%s: cannot convert %s to utf16 for mark of the web", local_path); - } - if (add_mark_of_web(filepath) == -1) { - remove(local_path); - free(filepath); fatal_f("%s: failed to add mark of the web", local_path); } - free(filepath); #endif // WINDOWS sshbuf_free(msg); free(handle); diff --git a/sftp-server.c b/sftp-server.c index 67b6d50edcc..8705faab7f0 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -867,9 +867,9 @@ process_write(u_int32_t id) fatal_fr(r, "parse"); #ifdef WINDOWS - wchar_t* filepath = resolved_path_utf16(handle_to_name(handle)); + char* filepath = resolved_path_utf8(handle_to_name(handle)); if (filepath == NULL) { - fatal_f("%s: cannot convert %s to utf16 for mark of the web", filepath); + fatal_f("cannot convert handle %d to utf8 filepath for mark of the web", handle); } if (add_mark_of_web(filepath) == -1) { fatal_f("%s: failed to add mark of the web", filepath); From b7487356ea5984c7876e9b8cd49b37631f69e568 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 4 Aug 2022 11:54:36 -0400 Subject: [PATCH 07/17] fix return value for null fileStreamPath pointer --- contrib/win32/win32compat/misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index b71847bd1dd..505e0132e87 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2132,7 +2132,7 @@ add_mark_of_web(const char* filename) fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); if (fileStreamPath == NULL) { - return status; + return -1; } // create zone identifer file stream and write the Mark of the Web to it sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); From b4b960505063cba53b22bbcff15d6b5cf18f9b9b Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 4 Aug 2022 15:10:23 -0400 Subject: [PATCH 08/17] fix spacing, comments, and failure message --- contrib/win32/win32compat/misc.c | 15 +++++++++++---- sftp-server.c | 4 +++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 505e0132e87..492a9104019 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2124,28 +2124,35 @@ strrstr(const char *inStr, const char *pattern) int add_mark_of_web(const char* filename) { - // ZoneId=3 indicates the file comes from the Internet Zone - const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; char* fileStreamPath = NULL; size_t fileStreamPathLen = strlen(filename) + strlen(":Zone.Identifier") + 1; - int ofd, status = 0; fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); + if (fileStreamPath == NULL) { return -1; } - // create zone identifer file stream and write the Mark of the Web to it + + // ZoneId=3 indicates the file comes from the Internet Zone + const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; + int ofd, status = 0; + sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); + + // create zone identifer file stream and then write the Mark of the Web to it if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { status = -1; goto cleanup; } + if (atomicio(vwrite, ofd, zoneIdentifier, strlen(zoneIdentifier)) != strlen(zoneIdentifier)) { status = -1; } + if (close(ofd) == -1) { status = -1; } + cleanup: free(fileStreamPath); return status; diff --git a/sftp-server.c b/sftp-server.c index 8705faab7f0..1519f182068 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -872,7 +872,9 @@ process_write(u_int32_t id) fatal_f("cannot convert handle %d to utf8 filepath for mark of the web", handle); } if (add_mark_of_web(filepath) == -1) { - fatal_f("%s: failed to add mark of the web", filepath); + debug("add_mark_of_web to %s failed", filepath); + free(filepath); + fatal_f("failed to add mark of the web"); } free(filepath); #endif // WINDOWS From ee6eeb82857ea6c7e35404dc4d7a458803972ab4 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 4 Aug 2022 17:12:18 -0400 Subject: [PATCH 09/17] clean up mark of the web method --- contrib/win32/win32compat/misc.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 492a9104019..b8a49308450 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2133,22 +2133,24 @@ add_mark_of_web(const char* filename) return -1; } + sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); + // ZoneId=3 indicates the file comes from the Internet Zone const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; int ofd, status = 0; - sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); - // create zone identifer file stream and then write the Mark of the Web to it if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { status = -1; goto cleanup; } - - if (atomicio(vwrite, ofd, zoneIdentifier, strlen(zoneIdentifier)) != strlen(zoneIdentifier)) { + + size_t zoneIndentifierLen = strlen(zoneIdentifier); + + if (atomicio(vwrite, ofd, zoneIdentifier, zoneIndentifierLen) != zoneIndentifierLen) { status = -1; } - + if (close(ofd) == -1) { status = -1; } From 77fab9d6487dac793565cf16ebfeab6063451610 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Wed, 17 Aug 2022 17:00:41 -0400 Subject: [PATCH 10/17] incorporate MapUrlToZone for sftp & partially scp --- contrib/win32/win32compat/misc.c | 60 +++++++++++++++++++++-- contrib/win32/win32compat/misc_internal.h | 4 ++ scp.c | 11 +++-- sftp-client.c | 8 +-- sftp-server.c | 14 ++++-- sftp.c | 3 ++ 6 files changed, 83 insertions(+), 17 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index b8a49308450..155b9d7aab4 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -61,6 +61,7 @@ #include "inc\string.h" #include "inc\time.h" #include "..\..\..\atomicio.h" +#include "urlmon.h" #include @@ -136,6 +137,9 @@ int chroot_path_len = 0; /* UTF-16 version of the above */ wchar_t* chroot_pathw = NULL; +/* motw zone_id initialized to invalid value */ +DWORD motw_zone_id = 5; + int usleep(unsigned int useconds) { @@ -2136,7 +2140,18 @@ add_mark_of_web(const char* filename) sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); // ZoneId=3 indicates the file comes from the Internet Zone - const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; + // const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; + char* zoneIdentifierStr = NULL; + size_t zoneIndentifierLen = strlen("[ZoneTransfer]\nZoneId=") + 1 + 1; + + zoneIdentifierStr = malloc(zoneIndentifierLen * sizeof(char)); + + if (zoneIdentifierStr == NULL) { + return -1; + } + + sprintf_s(zoneIdentifierStr, zoneIndentifierLen, "[ZoneTransfer]\nZoneId=%d", motw_zone_id); + int ofd, status = 0; // create zone identifer file stream and then write the Mark of the Web to it @@ -2145,9 +2160,9 @@ add_mark_of_web(const char* filename) goto cleanup; } - size_t zoneIndentifierLen = strlen(zoneIdentifier); + // size_t zoneIndentifierLen = strlen(zoneIdentifier); - if (atomicio(vwrite, ofd, zoneIdentifier, zoneIndentifierLen) != zoneIndentifierLen) { + if (atomicio(vwrite, ofd, zoneIdentifierStr, zoneIndentifierLen) != zoneIndentifierLen) { status = -1; } @@ -2160,3 +2175,42 @@ add_mark_of_web(const char* filename) return status; } +void get_zone_identifier(const char* hostname) { + static const CLSID CLSID_ISM = + { 0x7B8A2D94, 0x0AC9, 0x11D1, + { 0x89, 0x6C, 0x00, 0xC0, 0x4F, 0xB6, 0xBF, 0xC4 } }; + static const IID IID_IISM = + { 0x79EAC9EE, 0xBAF9, 0x11CE, + { 0x8C, 0x82, 0x00, 0xAA, 0x00, 0x4B, 0xA9, 0x0B } }; + IInternetSecurityManager* IISM; + CoInitialize(NULL); + HRESULT hr = CoCreateInstance(&CLSID_ISM, NULL, + CLSCTX_ALL, &IID_IISM, (void**)&IISM); + if (SUCCEEDED(hr) || hr == RPC_E_CHANGED_MODE) + { + wchar_t* hostname_w = utf8_to_utf16(hostname); + size_t inputStrLen = wcslen(hostname_w) + wcslen(L"ftp://") + 2; + wchar_t* inputStr = malloc(inputStrLen * sizeof(wchar_t)); + if (inputStr == NULL) { + return; + } + swprintf_s(inputStr, inputStrLen, L"ftp://%s", hostname_w); + hr = IISM->lpVtbl->MapUrlToZone(IISM, inputStr, &motw_zone_id, 0); + if (hr == S_OK) { + if (motw_zone_id < 5) { + debug("valid zone identifier value: %d", motw_zone_id); + } + else { + debug("invalid zone identifier value, will not use"); + motw_zone_id = 5; + } + } + else { + motw_zone_id = 5; + debug("MapUrlToZone failed"); + } + } + else { + debug("failed to co-create instance"); + } +} \ No newline at end of file diff --git a/contrib/win32/win32compat/misc_internal.h b/contrib/win32/win32compat/misc_internal.h index 73a487540ca..f6743229ccd 100644 --- a/contrib/win32/win32compat/misc_internal.h +++ b/contrib/win32/win32compat/misc_internal.h @@ -49,6 +49,9 @@ extern char* chroot_path; extern int chroot_path_len; extern wchar_t* chroot_pathw; +/* motw zone_id */ +extern DWORD motw_zone_id; + /* removes first '/' for Windows paths that are unix styled. Ex: /c:/ab.cd */ wchar_t * resolved_path_utf16(const char *); char* resolved_path_utf8(const char *); @@ -84,3 +87,4 @@ BOOL is_bash_test_env(); int bash_to_win_path(const char *in, char *out, const size_t out_len); void debug_assert_internal(); int add_mark_of_web(const char* filename); +void get_zone_identifier(const char* hostname); diff --git a/scp.c b/scp.c index 5fbdc279d4a..9d97028c0ed 100644 --- a/scp.c +++ b/scp.c @@ -1141,6 +1141,9 @@ do_sftp_connect(char *host, char *user, int port, char *sftp_direct, reminp, remoutp, pidp) < 0) return NULL; } +#ifdef WINDOWS + get_zone_identifier(host); +#endif // WINDOWS return do_init(*reminp, *remoutp, 32768, 64, limit_kbps); } @@ -1439,6 +1442,9 @@ tolocal(int argc, char **argv, enum scp_mode_e mode, char *sftp_direct) continue; } /* SCP */ +#ifdef WINDOWS + get_zone_identifier(host); +#endif // WINDOWS xasprintf(&bp, "%s -f %s%s", cmd, *src == '-' ? "-- " : "", src); if (do_cmd(ssh_program, host, suser, sport, 0, bp, @@ -2077,9 +2083,8 @@ sink(int argc, char **argv, const char *src) omode = mode; mode |= S_IWUSR; #ifdef WINDOWS - if (add_mark_of_web(np) == -1) { - run_err("%s: failed to add mark of the web\n", np); - exit(1); + if (motw_zone_id == 5 || add_mark_of_web(np) == -1) { + note_err("%s: failed to add mark of the web\n", np); } // In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX. diff --git a/sftp-client.c b/sftp-client.c index fefa35d31f6..b4d6a80ece7 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1728,12 +1728,8 @@ do_download(struct sftp_conn *conn, const char *remote_path, } close(local_fd); #ifdef WINDOWS - if (add_mark_of_web(local_path) == -1) { - // (resume_flag ? 0 : O_TRUNC) on line 1355 requires us - // to add the mark of the web after transferring the file. - // if MOTW fails, we need to remove the file before exiting - remove(local_path); - fatal_f("%s: failed to add mark of the web", local_path); + if (motw_zone_id == 5 || add_mark_of_web(local_path) == -1) { + debug("%s: failed to add mark of the web", local_path); } #endif // WINDOWS sshbuf_free(msg); diff --git a/sftp-server.c b/sftp-server.c index 1519f182068..32765a11f2b 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -869,14 +869,14 @@ process_write(u_int32_t id) #ifdef WINDOWS char* filepath = resolved_path_utf8(handle_to_name(handle)); if (filepath == NULL) { - fatal_f("cannot convert handle %d to utf8 filepath for mark of the web", handle); + debug("cannot convert handle %d to utf8 filepath for mark of the web", handle); } - if (add_mark_of_web(filepath) == -1) { - debug("add_mark_of_web to %s failed", filepath); + else { + if (motw_zone_id == 5 || add_mark_of_web(filepath) == -1) { + debug("add_mark_of_web to %s failed", filepath); + } free(filepath); - fatal_f("failed to add mark of the web"); } - free(filepath); #endif // WINDOWS debug("request %u: write \"%s\" (handle %d) off %llu len %zu", @@ -1913,6 +1913,10 @@ sftp_server_main(int argc, char **argv, struct passwd *user_pw) logit("session opened for local user %s from [%s]", pw->pw_name, client_addr); +#ifdef WINDOWS + get_zone_identifier(client_addr); +#endif // WINDOWS + in = STDIN_FILENO; out = STDOUT_FILENO; diff --git a/sftp.c b/sftp.c index a70ca27db50..8832350e235 100644 --- a/sftp.c +++ b/sftp.c @@ -2637,6 +2637,9 @@ main(int argc, char **argv) freeargs(&args); conn = do_init(in, out, copy_buffer_len, num_requests, limit_kbps); +#ifdef WINDOWS + get_zone_identifier(host); +#endif //WINDOWS if (conn == NULL) fatal("Couldn't initialise connection to server"); From 1e1973e73282f1f3f0c6f3f20e9ad54c705b7fd0 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Thu, 18 Aug 2022 15:20:43 -0400 Subject: [PATCH 11/17] update scp for motw failure case --- scp.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scp.c b/scp.c index 9d97028c0ed..dcdd23ba5af 100644 --- a/scp.c +++ b/scp.c @@ -2083,8 +2083,17 @@ sink(int argc, char **argv, const char *src) omode = mode; mode |= S_IWUSR; #ifdef WINDOWS - if (motw_zone_id == 5 || add_mark_of_web(np) == -1) { - note_err("%s: failed to add mark of the web\n", np); + if (motw_zone_id == 5) { + if (verbose_mode) + note_err("%s: will not add mark of the web due to push \ + from local to remote scenario, or MapUrlToZone API failure\n", np); + } + else { + if (add_mark_of_web(np) == -1) { + if (verbose_mode) { + note_err("%s: add_mark_of_web failed\n", np); + } + } } // In windows, we would like to inherit the parent folder permissions by setting mode to USHRT_MAX. From 967d9f46484bf4e4851cdb0f3af8e2ebdaa9ab39 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Fri, 19 Aug 2022 10:02:08 -0400 Subject: [PATCH 12/17] Update contrib/win32/win32compat/misc.c Co-authored-by: Paul Higinbotham --- contrib/win32/win32compat/misc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 155b9d7aab4..0ec4d2cdcfd 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2175,6 +2175,7 @@ add_mark_of_web(const char* filename) return status; } +/* Gets the zone identifier value based on the provided hostname, and sets the global motw_zone_id variable with that value. */ void get_zone_identifier(const char* hostname) { static const CLSID CLSID_ISM = { 0x7B8A2D94, 0x0AC9, 0x11D1, From b769ecc2a79a4660c63ed51422ff730dedf8840c Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Fri, 19 Aug 2022 13:49:31 -0400 Subject: [PATCH 13/17] address pr review comments --- contrib/win32/win32compat/misc.c | 97 ++++++++++++++++---------------- scp.c | 13 +---- sftp-client.c | 2 +- sftp-server.c | 2 +- 4 files changed, 55 insertions(+), 59 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 0ec4d2cdcfd..adc8f373d5e 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2128,54 +2128,57 @@ strrstr(const char *inStr, const char *pattern) int add_mark_of_web(const char* filename) { - char* fileStreamPath = NULL; - size_t fileStreamPathLen = strlen(filename) + strlen(":Zone.Identifier") + 1; + if (motw_zone_id < 5) { + char* fileStreamPath = NULL; + size_t fileStreamPathLen = strlen(filename) + strlen(":Zone.Identifier") + 1; - fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); + fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); - if (fileStreamPath == NULL) { - return -1; - } + if (fileStreamPath == NULL) { + return -1; + } - sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); + sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); - // ZoneId=3 indicates the file comes from the Internet Zone - // const char zoneIdentifier[] = "[ZoneTransfer]\nZoneId=3"; - char* zoneIdentifierStr = NULL; - size_t zoneIndentifierLen = strlen("[ZoneTransfer]\nZoneId=") + 1 + 1; + char* zoneIdentifierStr = NULL; + size_t zoneIndentifierLen = strlen("[ZoneTransfer]\nZoneId=") + 1 + 1; - zoneIdentifierStr = malloc(zoneIndentifierLen * sizeof(char)); + zoneIdentifierStr = malloc(zoneIndentifierLen * sizeof(char)); - if (zoneIdentifierStr == NULL) { - return -1; - } + if (zoneIdentifierStr == NULL) { + return -1; + } - sprintf_s(zoneIdentifierStr, zoneIndentifierLen, "[ZoneTransfer]\nZoneId=%d", motw_zone_id); + sprintf_s(zoneIdentifierStr, zoneIndentifierLen, "[ZoneTransfer]\nZoneId=%d", motw_zone_id); - int ofd, status = 0; + int ofd, status = 0; - // create zone identifer file stream and then write the Mark of the Web to it - if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { - status = -1; - goto cleanup; - } + // create zone identifer file stream and then write the Mark of the Web to it + if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { + status = -1; + goto cleanup; + } - // size_t zoneIndentifierLen = strlen(zoneIdentifier); + if (atomicio(vwrite, ofd, zoneIdentifierStr, zoneIndentifierLen) != zoneIndentifierLen) { + status = -1; + } - if (atomicio(vwrite, ofd, zoneIdentifierStr, zoneIndentifierLen) != zoneIndentifierLen) { - status = -1; - } + if (close(ofd) == -1) { + status = -1; + } - if (close(ofd) == -1) { - status = -1; + cleanup: + free(fileStreamPath); + free(zoneIdentifierStr); + return status; + } + else { + return -1; } - -cleanup: - free(fileStreamPath); - return status; } -/* Gets the zone identifier value based on the provided hostname, and sets the global motw_zone_id variable with that value. */ +/* Gets the zone identifier value based on the provided hostname, +and sets the global motw_zone_id variable with that value. */ void get_zone_identifier(const char* hostname) { static const CLSID CLSID_ISM = { 0x7B8A2D94, 0x0AC9, 0x11D1, @@ -2190,28 +2193,28 @@ void get_zone_identifier(const char* hostname) { if (SUCCEEDED(hr) || hr == RPC_E_CHANGED_MODE) { wchar_t* hostname_w = utf8_to_utf16(hostname); - size_t inputStrLen = wcslen(hostname_w) + wcslen(L"ftp://") + 2; - wchar_t* inputStr = malloc(inputStrLen * sizeof(wchar_t)); - if (inputStr == NULL) { + if (hostname_w == NULL) { + return; + } + size_t host_format_len = wcslen(hostname_w) + wcslen(L"ftp://") + 1; + wchar_t* host_format = malloc(host_format_len * sizeof(wchar_t)); + if (host_format == NULL) { + free(hostname_w); return; } - swprintf_s(inputStr, inputStrLen, L"ftp://%s", hostname_w); - hr = IISM->lpVtbl->MapUrlToZone(IISM, inputStr, &motw_zone_id, 0); + swprintf_s(host_format, host_format_len, L"ftp://%s", hostname_w); + hr = IISM->lpVtbl->MapUrlToZone(IISM, host_format, &motw_zone_id, 0); if (hr == S_OK) { - if (motw_zone_id < 5) { - debug("valid zone identifier value: %d", motw_zone_id); - } - else { - debug("invalid zone identifier value, will not use"); - motw_zone_id = 5; - } + debug("MapUrlToZone zone identifier value: %d", motw_zone_id); } else { motw_zone_id = 5; - debug("MapUrlToZone failed"); + debug("MapUrlToZone failed, resetting motw_zone_id to invalid value"); } + free(hostname_w); + free(host_format); } else { - debug("failed to co-create instance"); + debug("failed to co-create instance for MapUrlToZone"); } } \ No newline at end of file diff --git a/scp.c b/scp.c index dcdd23ba5af..67152f50b34 100644 --- a/scp.c +++ b/scp.c @@ -2083,16 +2083,9 @@ sink(int argc, char **argv, const char *src) omode = mode; mode |= S_IWUSR; #ifdef WINDOWS - if (motw_zone_id == 5) { - if (verbose_mode) - note_err("%s: will not add mark of the web due to push \ - from local to remote scenario, or MapUrlToZone API failure\n", np); - } - else { - if (add_mark_of_web(np) == -1) { - if (verbose_mode) { - note_err("%s: add_mark_of_web failed\n", np); - } + if (add_mark_of_web(np) == -1) { + if (verbose_mode) { + note_err("%s: add_mark_of_web failed\n", np); } } diff --git a/sftp-client.c b/sftp-client.c index b4d6a80ece7..de8e54e11c0 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1728,7 +1728,7 @@ do_download(struct sftp_conn *conn, const char *remote_path, } close(local_fd); #ifdef WINDOWS - if (motw_zone_id == 5 || add_mark_of_web(local_path) == -1) { + if (add_mark_of_web(local_path) == -1) { debug("%s: failed to add mark of the web", local_path); } #endif // WINDOWS diff --git a/sftp-server.c b/sftp-server.c index 32765a11f2b..1efd2ae2961 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -872,7 +872,7 @@ process_write(u_int32_t id) debug("cannot convert handle %d to utf8 filepath for mark of the web", handle); } else { - if (motw_zone_id == 5 || add_mark_of_web(filepath) == -1) { + if (add_mark_of_web(filepath) == -1) { debug("add_mark_of_web to %s failed", filepath); } free(filepath); From ef40c616d2565e0014d968edc5425faf3aa78dd4 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Mon, 22 Aug 2022 13:47:20 -0400 Subject: [PATCH 14/17] refactor failure handling in motw methods --- contrib/win32/win32compat/misc.c | 116 +++++++++++++++---------------- 1 file changed, 57 insertions(+), 59 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index adc8f373d5e..3750170d783 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2128,53 +2128,52 @@ strrstr(const char *inStr, const char *pattern) int add_mark_of_web(const char* filename) { - if (motw_zone_id < 5) { - char* fileStreamPath = NULL; - size_t fileStreamPathLen = strlen(filename) + strlen(":Zone.Identifier") + 1; - - fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); + if (motw_zone_id > 4) { + return -1; + } + char* fileStreamPath = NULL; + size_t fileStreamPathLen = strlen(filename) + strlen(":Zone.Identifier") + 1; - if (fileStreamPath == NULL) { - return -1; - } + fileStreamPath = malloc(fileStreamPathLen * sizeof(char)); - sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); + if (fileStreamPath == NULL) { + return -1; + } - char* zoneIdentifierStr = NULL; - size_t zoneIndentifierLen = strlen("[ZoneTransfer]\nZoneId=") + 1 + 1; + sprintf_s(fileStreamPath, fileStreamPathLen, "%s:Zone.Identifier", filename); - zoneIdentifierStr = malloc(zoneIndentifierLen * sizeof(char)); + int ofd, status = 0; + char* zoneIdentifierStr = NULL; + size_t zoneIndentifierLen = strlen("[ZoneTransfer]\nZoneId=") + 1 + 1; - if (zoneIdentifierStr == NULL) { - return -1; - } + zoneIdentifierStr = malloc(zoneIndentifierLen * sizeof(char)); - sprintf_s(zoneIdentifierStr, zoneIndentifierLen, "[ZoneTransfer]\nZoneId=%d", motw_zone_id); + if (zoneIdentifierStr == NULL) { + status = -1; + goto cleanup; + } - int ofd, status = 0; + sprintf_s(zoneIdentifierStr, zoneIndentifierLen, "[ZoneTransfer]\nZoneId=%d", motw_zone_id); - // create zone identifer file stream and then write the Mark of the Web to it - if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { - status = -1; - goto cleanup; - } + // create zone identifer file stream and then write the Mark of the Web to it + if ((ofd = open(fileStreamPath, O_WRONLY | O_CREAT, USHRT_MAX)) == -1) { + status = -1; + goto cleanup; + } - if (atomicio(vwrite, ofd, zoneIdentifierStr, zoneIndentifierLen) != zoneIndentifierLen) { - status = -1; - } + if (atomicio(vwrite, ofd, zoneIdentifierStr, zoneIndentifierLen) != zoneIndentifierLen) { + status = -1; + } - if (close(ofd) == -1) { - status = -1; - } + if (close(ofd) == -1) { + status = -1; + } - cleanup: - free(fileStreamPath); +cleanup: + free(fileStreamPath); + if (zoneIdentifierStr) free(zoneIdentifierStr); - return status; - } - else { - return -1; - } + return status; } /* Gets the zone identifier value based on the provided hostname, @@ -2190,31 +2189,30 @@ void get_zone_identifier(const char* hostname) { CoInitialize(NULL); HRESULT hr = CoCreateInstance(&CLSID_ISM, NULL, CLSCTX_ALL, &IID_IISM, (void**)&IISM); - if (SUCCEEDED(hr) || hr == RPC_E_CHANGED_MODE) - { - wchar_t* hostname_w = utf8_to_utf16(hostname); - if (hostname_w == NULL) { - return; - } - size_t host_format_len = wcslen(hostname_w) + wcslen(L"ftp://") + 1; - wchar_t* host_format = malloc(host_format_len * sizeof(wchar_t)); - if (host_format == NULL) { - free(hostname_w); - return; - } - swprintf_s(host_format, host_format_len, L"ftp://%s", hostname_w); - hr = IISM->lpVtbl->MapUrlToZone(IISM, host_format, &motw_zone_id, 0); - if (hr == S_OK) { - debug("MapUrlToZone zone identifier value: %d", motw_zone_id); - } - else { - motw_zone_id = 5; - debug("MapUrlToZone failed, resetting motw_zone_id to invalid value"); - } - free(hostname_w); - free(host_format); + if (!(SUCCEEDED(hr) || hr == RPC_E_CHANGED_MODE)) { + motw_zone_id = 5; + debug("failed to co-create instance for MapUrlToZone"); + return; + } + wchar_t* hostname_w = utf8_to_utf16(hostname); + if (hostname_w == NULL) { + return; + } + size_t host_format_len = wcslen(hostname_w) + wcslen(L"ftp://") + 1; + wchar_t* host_format = malloc(host_format_len * sizeof(wchar_t)); + if (host_format == NULL) { + goto cleanup; + } + swprintf_s(host_format, host_format_len, L"ftp://%s", hostname_w); + hr = IISM->lpVtbl->MapUrlToZone(IISM, host_format, &motw_zone_id, 0); + if (hr == S_OK) { + debug("MapUrlToZone zone identifier value: %d", motw_zone_id); } else { - debug("failed to co-create instance for MapUrlToZone"); + motw_zone_id = 5; + debug("MapUrlToZone failed, resetting motw_zone_id to invalid value"); } + free(host_format); +cleanup: + free(hostname_w); } \ No newline at end of file From d78c66df18ef1a7adfef9e8aedf8fad61d6161a1 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Mon, 22 Aug 2022 18:27:37 -0400 Subject: [PATCH 15/17] add CoUnitialize after CoInitializeEx call --- contrib/win32/win32compat/misc.c | 33 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 3750170d783..cdc5163bafd 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2179,27 +2179,31 @@ add_mark_of_web(const char* filename) /* Gets the zone identifier value based on the provided hostname, and sets the global motw_zone_id variable with that value. */ void get_zone_identifier(const char* hostname) { + HRESULT hrCoInit = CoInitializeEx(NULL, COINIT_MULTITHREADED); + if (!(SUCCEEDED(hrCoInit) || hrCoInit == RPC_E_CHANGED_MODE)) { + debug("CoInitializeEx for MapUrlToZone failed"); + return; + } static const CLSID CLSID_ISM = { 0x7B8A2D94, 0x0AC9, 0x11D1, { 0x89, 0x6C, 0x00, 0xC0, 0x4F, 0xB6, 0xBF, 0xC4 } }; static const IID IID_IISM = { 0x79EAC9EE, 0xBAF9, 0x11CE, { 0x8C, 0x82, 0x00, 0xAA, 0x00, 0x4B, 0xA9, 0x0B } }; - IInternetSecurityManager* IISM; - CoInitialize(NULL); + IInternetSecurityManager* IISM = NULL; HRESULT hr = CoCreateInstance(&CLSID_ISM, NULL, CLSCTX_ALL, &IID_IISM, (void**)&IISM); - if (!(SUCCEEDED(hr) || hr == RPC_E_CHANGED_MODE)) { - motw_zone_id = 5; - debug("failed to co-create instance for MapUrlToZone"); - return; + if (!SUCCEEDED(hr)) { + debug("CoCreateInstance for MapUrlToZone failed"); + goto out; } - wchar_t* hostname_w = utf8_to_utf16(hostname); + wchar_t *hostname_w = NULL, *host_format = NULL; + hostname_w = utf8_to_utf16(hostname); if (hostname_w == NULL) { - return; + goto cleanup; } size_t host_format_len = wcslen(hostname_w) + wcslen(L"ftp://") + 1; - wchar_t* host_format = malloc(host_format_len * sizeof(wchar_t)); + host_format = malloc(host_format_len * sizeof(wchar_t)); if (host_format == NULL) { goto cleanup; } @@ -2212,7 +2216,14 @@ void get_zone_identifier(const char* hostname) { motw_zone_id = 5; debug("MapUrlToZone failed, resetting motw_zone_id to invalid value"); } - free(host_format); cleanup: - free(hostname_w); + if (IISM) + IISM->lpVtbl->Release(IISM); + if (hostname_w) + free(hostname_w); + if (host_format) + free(host_format); +out: + if (SUCCEEDED(hrCoInit)) + CoUninitialize(); } \ No newline at end of file From 56bb9d57cd176cebdcc0a66f8256b627cb0c576b Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 23 Aug 2022 11:24:38 -0400 Subject: [PATCH 16/17] use urlmon.h constants for com instance --- contrib/win32/win32compat/misc.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index cdc5163bafd..0b6f2f28d90 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2179,20 +2179,15 @@ add_mark_of_web(const char* filename) /* Gets the zone identifier value based on the provided hostname, and sets the global motw_zone_id variable with that value. */ void get_zone_identifier(const char* hostname) { - HRESULT hrCoInit = CoInitializeEx(NULL, COINIT_MULTITHREADED); - if (!(SUCCEEDED(hrCoInit) || hrCoInit == RPC_E_CHANGED_MODE)) { + HRESULT hr = CoInitializeEx(NULL, COINIT_MULTITHREADED); + if (!SUCCEEDED(hr)) { debug("CoInitializeEx for MapUrlToZone failed"); return; } - static const CLSID CLSID_ISM = - { 0x7B8A2D94, 0x0AC9, 0x11D1, - { 0x89, 0x6C, 0x00, 0xC0, 0x4F, 0xB6, 0xBF, 0xC4 } }; - static const IID IID_IISM = - { 0x79EAC9EE, 0xBAF9, 0x11CE, - { 0x8C, 0x82, 0x00, 0xAA, 0x00, 0x4B, 0xA9, 0x0B } }; - IInternetSecurityManager* IISM = NULL; - HRESULT hr = CoCreateInstance(&CLSID_ISM, NULL, - CLSCTX_ALL, &IID_IISM, (void**)&IISM); + IInternetSecurityManager* pIISM = NULL; + // CLSID_InternetSecurityManager & IID_IInternetSecurityManager declared in urlmon.h + hr = CoCreateInstance(&CLSID_InternetSecurityManager, NULL, + CLSCTX_ALL, &IID_IInternetSecurityManager, (void**)&pIISM); if (!SUCCEEDED(hr)) { debug("CoCreateInstance for MapUrlToZone failed"); goto out; @@ -2208,7 +2203,7 @@ void get_zone_identifier(const char* hostname) { goto cleanup; } swprintf_s(host_format, host_format_len, L"ftp://%s", hostname_w); - hr = IISM->lpVtbl->MapUrlToZone(IISM, host_format, &motw_zone_id, 0); + hr = pIISM->lpVtbl->MapUrlToZone(pIISM, host_format, &motw_zone_id, 0); if (hr == S_OK) { debug("MapUrlToZone zone identifier value: %d", motw_zone_id); } @@ -2217,13 +2212,12 @@ void get_zone_identifier(const char* hostname) { debug("MapUrlToZone failed, resetting motw_zone_id to invalid value"); } cleanup: - if (IISM) - IISM->lpVtbl->Release(IISM); + if (pIISM) + pIISM->lpVtbl->Release(pIISM); if (hostname_w) free(hostname_w); if (host_format) free(host_format); out: - if (SUCCEEDED(hrCoInit)) - CoUninitialize(); + CoUninitialize(); } \ No newline at end of file From fc434133ee06ac74a5152d7c9481cb31d80ca072 Mon Sep 17 00:00:00 2001 From: Tess Gauthier Date: Tue, 23 Aug 2022 13:20:07 -0400 Subject: [PATCH 17/17] update var name for consistency --- contrib/win32/win32compat/misc.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contrib/win32/win32compat/misc.c b/contrib/win32/win32compat/misc.c index 0b6f2f28d90..f20ad0e4e86 100644 --- a/contrib/win32/win32compat/misc.c +++ b/contrib/win32/win32compat/misc.c @@ -2184,7 +2184,7 @@ void get_zone_identifier(const char* hostname) { debug("CoInitializeEx for MapUrlToZone failed"); return; } - IInternetSecurityManager* pIISM = NULL; + IInternetSecurityManager *pIISM = NULL; // CLSID_InternetSecurityManager & IID_IInternetSecurityManager declared in urlmon.h hr = CoCreateInstance(&CLSID_InternetSecurityManager, NULL, CLSCTX_ALL, &IID_IInternetSecurityManager, (void**)&pIISM); @@ -2192,18 +2192,18 @@ void get_zone_identifier(const char* hostname) { debug("CoCreateInstance for MapUrlToZone failed"); goto out; } - wchar_t *hostname_w = NULL, *host_format = NULL; + wchar_t *hostname_w = NULL, *hostformat_w = NULL; hostname_w = utf8_to_utf16(hostname); if (hostname_w == NULL) { goto cleanup; } - size_t host_format_len = wcslen(hostname_w) + wcslen(L"ftp://") + 1; - host_format = malloc(host_format_len * sizeof(wchar_t)); - if (host_format == NULL) { + size_t hostname_w_len = wcslen(hostname_w) + wcslen(L"ftp://") + 1; + hostformat_w = malloc(hostname_w_len * sizeof(wchar_t)); + if (hostformat_w == NULL) { goto cleanup; } - swprintf_s(host_format, host_format_len, L"ftp://%s", hostname_w); - hr = pIISM->lpVtbl->MapUrlToZone(pIISM, host_format, &motw_zone_id, 0); + swprintf_s(hostformat_w, hostname_w_len, L"ftp://%s", hostname_w); + hr = pIISM->lpVtbl->MapUrlToZone(pIISM, hostformat_w, &motw_zone_id, 0); if (hr == S_OK) { debug("MapUrlToZone zone identifier value: %d", motw_zone_id); } @@ -2216,8 +2216,8 @@ void get_zone_identifier(const char* hostname) { pIISM->lpVtbl->Release(pIISM); if (hostname_w) free(hostname_w); - if (host_format) - free(host_format); + if (hostformat_w) + free(hostformat_w); out: CoUninitialize(); -} \ No newline at end of file +}