From 269ea8aaf5d3fc2b9e577b09e217dc0ed051dd2b Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 16:57:51 +0000 Subject: [PATCH 01/16] Move predeclaration of struct unicode_data into defs.h. It's just the sort of thing that ought to be in there, once, so it doesn't have to be declared in n places. --- defs.h | 2 ++ unix/platform.h | 1 - windows/platform.h | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/defs.h b/defs.h index 45bc90f3..3c30b6b6 100644 --- a/defs.h +++ b/defs.h @@ -192,6 +192,8 @@ typedef struct logblank_t logblank_t; typedef struct BinaryPacketProtocol BinaryPacketProtocol; typedef struct PacketProtocolLayer PacketProtocolLayer; +struct unicode_data; + /* Do a compile-time type-check of 'to_check' (without evaluating it), * as a side effect of returning the value 'to_return'. Note that * although this macro double-*expands* to_return, it always diff --git a/unix/platform.h b/unix/platform.h index 6dff7842..7cdf4d11 100644 --- a/unix/platform.h +++ b/unix/platform.h @@ -347,7 +347,6 @@ char *make_dir_path(const char *path, mode_t mode); /* * Exports from unicode.c. */ -struct unicode_data; bool init_ucs(struct unicode_data *ucsdata, char *line_codepage, bool utf8_override, int font_charset, int vtmode); diff --git a/windows/platform.h b/windows/platform.h index 6900a3c6..d15e1a68 100644 --- a/windows/platform.h +++ b/windows/platform.h @@ -606,7 +606,6 @@ void EnableSizeTip(bool bEnable); /* * Exports from unicode.c. */ -struct unicode_data; void init_ucs(Conf *, struct unicode_data *); /* From 21f602be407838a53b281484afdeb068a84713b8 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 16:01:21 +0000 Subject: [PATCH 02/16] Add utility function dup_wc_to_mb. This parallels dup_mb_to_wc, which already existed. I haven't needed the same thing this way round yet, but I'm about to. --- misc.h | 4 ++++ utils/CMakeLists.txt | 1 + utils/dup_wc_to_mb.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 utils/dup_wc_to_mb.c diff --git a/misc.h b/misc.h index a85b87ae..a78f8c8d 100644 --- a/misc.h +++ b/misc.h @@ -67,6 +67,10 @@ void strbuf_finalise_agent_query(strbuf *buf); * work around the rather deficient interface of mb_to_wc. */ wchar_t *dup_mb_to_wc_c(int codepage, int flags, const char *string, int len); wchar_t *dup_mb_to_wc(int codepage, int flags, const char *string); +char *dup_wc_to_mb_c(int codepage, int flags, const wchar_t *string, int len, + const char *defchr, struct unicode_data *ucsdata); +char *dup_wc_to_mb(int codepage, int flags, const wchar_t *string, + const char *defchr, struct unicode_data *ucsdata); static inline int toint(unsigned u) { diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 4a6a1fe8..8048f811 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -17,6 +17,7 @@ add_sources_from_current_dir(utils dupprintf.c dupstr.c dup_mb_to_wc.c + dup_wc_to_mb.c encode_utf8.c encode_wide_string_as_utf8.c fgetline.c diff --git a/utils/dup_wc_to_mb.c b/utils/dup_wc_to_mb.c new file mode 100644 index 00000000..e91a8916 --- /dev/null +++ b/utils/dup_wc_to_mb.c @@ -0,0 +1,39 @@ +/* + * dup_wc_to_mb: memory-allocating wrapper on wc_to_mb. + * + * Also dup_wc_to_mb_c: same but you already know the length of the + * string. + */ + +#include + +#include "putty.h" +#include "misc.h" + +char *dup_wc_to_mb_c(int codepage, int flags, const wchar_t *string, int len, + const char *defchr, struct unicode_data *ucsdata) +{ + size_t outsize = len+1; + char *out = snewn(outsize, char); + + while (true) { + size_t outlen = wc_to_mb(codepage, flags, string, len, out, outsize, + defchr, ucsdata); + /* We can only be sure we've consumed the whole input if the + * output is not within a multibyte-character-length of the + * end of the buffer! */ + if (outlen < outsize && outsize - outlen > MB_LEN_MAX) { + out[outlen] = '\0'; + return out; + } + + sgrowarray(out, outsize, outsize); + } +} + +char *dup_wc_to_mb(int codepage, int flags, const wchar_t *string, + const char *defchr, struct unicode_data *ucsdata) +{ + return dup_wc_to_mb_c(codepage, flags, string, wcslen(string), + defchr, ucsdata); +} From b360ea6ac19b7688ab0a8b09350298874b731c4a Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 15:53:04 +0000 Subject: [PATCH 03/16] Add a manual single-char UTF-8 decoder. This parallels encode_utf8 which we already had. Decoding is more fraught with perils than encoding, so I've also included a small test program. --- CMakeLists.txt | 5 + misc.h | 11 +++ utils/CMakeLists.txt | 2 + utils/decode_utf8.c | 178 +++++++++++++++++++++++++++++++++++ utils/decode_utf8_to_wchar.c | 20 ++++ 5 files changed, 216 insertions(+) create mode 100644 utils/decode_utf8.c create mode 100644 utils/decode_utf8_to_wchar.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 3aba5e20..0eb4cf1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -79,6 +79,11 @@ add_executable(test_host_strfoo target_compile_definitions(test_host_strfoo PRIVATE TEST) target_link_libraries(test_host_strfoo utils ${platform_libraries}) +add_executable(test_decode_utf8 + utils/decode_utf8.c) +target_compile_definitions(test_decode_utf8 PRIVATE TEST) +target_link_libraries(test_decode_utf8 utils ${platform_libraries}) + add_executable(test_tree234 utils/tree234.c) target_compile_definitions(test_tree234 PRIVATE TEST) diff --git a/misc.h b/misc.h index a78f8c8d..dea7190b 100644 --- a/misc.h +++ b/misc.h @@ -221,6 +221,17 @@ size_t encode_utf8(void *output, unsigned long ch); * encoded in UTF-16. */ char *encode_wide_string_as_utf8(const wchar_t *wstr); +/* Decode a single UTF-8 character. Returns U+FFFD for any of the + * illegal cases. */ +unsigned long decode_utf8(const char **utf8); + +/* Decode a single UTF-8 character to an output buffer of the + * platform's wchar_t. May write a pair of surrogates if + * sizeof(wchar_t) == 2, assuming that in that case the wide string is + * encoded in UTF-16. Otherwise, writes one character. Returns the + * number written. */ +size_t decode_utf8_to_wchar(const char **utf8, wchar_t *out); + /* Write a string out in C string-literal format. */ void write_c_string_literal(FILE *fp, ptrlen str); diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 8048f811..80fc20b8 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -12,6 +12,8 @@ add_sources_from_current_dir(utils conf_launchable.c ctrlparse.c debug.c + decode_utf8.c + decode_utf8_to_wchar.c default_description.c dupcat.c dupprintf.c diff --git a/utils/decode_utf8.c b/utils/decode_utf8.c new file mode 100644 index 00000000..c8dbec79 --- /dev/null +++ b/utils/decode_utf8.c @@ -0,0 +1,178 @@ +/* + * Decode a single UTF-8 character. + */ + +#include "putty.h" +#include "misc.h" + +unsigned long decode_utf8(const char **utf8) +{ + unsigned char c = (unsigned char)*(*utf8)++; + + /* One-byte cases. */ + if (c < 0x80) { + return c; + } else if (c < 0xC0) { + return 0xFFFD; /* spurious continuation byte */ + } + + unsigned long wc, min; + size_t ncont; + if (c < 0xE0) { + wc = c & 0x1F; ncont = 1; min = 0x80; + } else if (c < 0xF0) { + wc = c & 0x0F; ncont = 2; min = 0x800; + } else if (c < 0xF8) { + wc = c & 0x07; ncont = 3; min = 0x10000; + } else if (c < 0xFC) { + wc = c & 0x03; ncont = 4; min = 0x200000; + } else if (c < 0xFE) { + wc = c & 0x01; ncont = 5; min = 0x4000000; + } else { + return 0xFFFD; /* FE or FF illegal bytes */ + } + + while (ncont-- > 0) { + unsigned char cont = (unsigned char)**utf8; + if (!(0x80 <= cont && cont < 0xC0)) + return 0xFFFD; /* short sequence */ + (*utf8)++; + + wc = (wc << 6) | (cont & 0x3F); + } + + if (wc < min) + return 0xFFFD; /* overlong encoding */ + if (0xD800 <= wc && wc < 0xE000) + return 0xFFFD; /* UTF-8 encoding of surrogate */ + if (wc > 0x10FFFF) + return 0xFFFD; /* outside Unicode range */ + return wc; +} + +#ifdef TEST + +#include + +bool dotest(const char *file, int line, const char *input, + const unsigned long *chars, size_t nchars) +{ + const char *start = input; + const char *end = input + strlen(input) + 1; + size_t noutput = 0; + + printf("%s:%d: test start\n", file, line); + + while (input < end) { + const char *before = input; + unsigned long wc = decode_utf8(&input); + + printf("%s:%d in+%"SIZEu" out+%"SIZEu":", + file, line, (size_t)(before-start), noutput); + while (before < input) + printf(" %02x", (unsigned)(unsigned char)(*before++)); + printf(" -> U-%08lx\n", wc); + + if (noutput >= nchars) { + printf("%s:%d: FAIL: expected no further output\n", file, line); + return false; + } + + if (chars[noutput] != wc) { + printf("%s:%d: FAIL: expected U-%08lx\n", + file, line, chars[noutput]); + return false; + } + + noutput++; + } + + if (noutput < nchars) { + printf("%s:%d: FAIL: expected further output\n", file, line); + return false; + } + + printf("%s:%d: pass\n", file, line); + return true; +} + +#define DOTEST(input, ...) do { \ + static const unsigned long chars[] = { __VA_ARGS__, 0 }; \ + ntest++; \ + if (dotest(__FILE__, __LINE__, input, chars, lenof(chars))) \ + npass++; \ + } while (0) + +int main(void) +{ + int ntest = 0, npass = 0; + + DOTEST("\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5", + 0x03BA, 0x1F79, 0x03C3, 0x03BC, 0x03B5); + + /* First sequence of each length (not counting NUL, which is + * tested anyway by the string-termination handling in every test) */ + DOTEST("\xC2\x80", 0x0080); + DOTEST("\xE0\xA0\x80", 0x0800); + DOTEST("\xF0\x90\x80\x80", 0x00010000); + DOTEST("\xF8\x88\x80\x80\x80", 0xFFFD); /* would be 0x00200000 */ + DOTEST("\xFC\x84\x80\x80\x80\x80", 0xFFFD); /* would be 0x04000000 */ + + /* Last sequence of each length */ + DOTEST("\x7F", 0x007F); + DOTEST("\xDF\xBF", 0x07FF); + DOTEST("\xEF\xBF\xBF", 0xFFFF); + DOTEST("\xF7\xBF\xBF\xBF", 0xFFFD); /* would be 0x001FFFFF */ + DOTEST("\xFB\xBF\xBF\xBF\xBF", 0xFFFD); /* would be 0x03FFFFFF */ + DOTEST("\xFD\xBF\xBF\xBF\xBF\xBF", 0xFFFD); /* would be 0x7FFFFFFF */ + + /* Endpoints of the surrogate range */ + DOTEST("\xED\x9F\xBF", 0xD7FF); + DOTEST("\xED\xA0\x00", 0xFFFD); /* would be 0xD800 */ + DOTEST("\xED\xBF\xBF", 0xFFFD); /* would be 0xDFFF */ + DOTEST("\xEE\x80\x80", 0xE000); + + /* REPLACEMENT CHARACTER itself */ + DOTEST("\xEF\xBF\xBD", 0xFFFD); + + /* Endpoints of the legal Unicode range */ + DOTEST("\xF4\x8F\xBF\xBF", 0x0010FFFF); + DOTEST("\xF4\x90\x80\x80", 0xFFFD); /* would be 0x00110000 */ + + /* Spurious continuation bytes, each shown as a separate failure */ + DOTEST("\x80 \x81\x82 \xBD\xBE\xBF", + 0xFFFD, 0x0020, 0xFFFD, 0xFFFD, 0x0020, 0xFFFD, 0xFFFD, 0xFFFD); + + /* Truncated sequences, each shown as just one failure */ + DOTEST("\xC2\xE0\xA0\xF0\x90\x80\xF8\x88\x80\x80\xFC\x84\x80\x80\x80", + 0xFFFD, 0xFFFD, 0xFFFD, 0xFFFD, 0xFFFD); + DOTEST("\xC2 \xE0\xA0 \xF0\x90\x80 \xF8\x88\x80\x80 \xFC\x84\x80\x80\x80", + 0xFFFD, 0x0020, 0xFFFD, 0x0020, 0xFFFD, 0x0020, 0xFFFD, 0x0020, + 0xFFFD); + + /* Illegal bytes */ + DOTEST("\xFE\xFF", 0xFFFD, 0xFFFD); + + /* Overlong sequences */ + DOTEST("\xC1\xBF", 0xFFFD); + DOTEST("\xE0\x9F\xBF", 0xFFFD); + DOTEST("\xF0\x8F\xBF\xBF", 0xFFFD); + DOTEST("\xF8\x87\xBF\xBF\xBF", 0xFFFD); + DOTEST("\xFC\x83\xBF\xBF\xBF\xBF", 0xFFFD); + + DOTEST("\xC0\x80", 0xFFFD); + DOTEST("\xE0\x80\x80", 0xFFFD); + DOTEST("\xF0\x80\x80\x80", 0xFFFD); + DOTEST("\xF8\x80\x80\x80\x80", 0xFFFD); + DOTEST("\xFC\x80\x80\x80\x80\x80", 0xFFFD); + + printf("%d tests %d passed", ntest, npass); + if (npass < ntest) { + printf(" %d FAILED\n", ntest-npass); + return 1; + } else { + printf("\n"); + return 0; + } +} +#endif diff --git a/utils/decode_utf8_to_wchar.c b/utils/decode_utf8_to_wchar.c new file mode 100644 index 00000000..97a83218 --- /dev/null +++ b/utils/decode_utf8_to_wchar.c @@ -0,0 +1,20 @@ +/* + * Decode a single UTF-8 character to the platform's local wchar_t. + */ + +#include "putty.h" +#include "misc.h" + +size_t decode_utf8_to_wchar(const char **utf8, wchar_t *out) +{ + size_t outlen = 0; + unsigned wc = decode_utf8(utf8); + if (sizeof(wchar_t) > 2 || wc < 0x10000) { + out[outlen++] = wc; + } else { + unsigned wcoff = wc - 0x10000; + out[outlen++] = 0xD800 | (0x3FF & (wcoff >> 10)); + out[outlen++] = 0xDC00 | (0x3FF & wcoff); + } + return outlen; +} From cf41bc0c624eca01d5a710bee46f63e631b61aae Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 18:47:27 +0000 Subject: [PATCH 04/16] Unix mb_to_wc: add missing bounds checks. Checking various implementations of these functions against each other, I noticed by eyeball review that some of the special cases in mb_to_wc() never check the buffer limit at all. Yikes! Fortunately, I think there's no vulnerability, because these special cases are ones that write out at most one wide char per multibyte char, and at all the call sites (including dup_mb_to_wc) we allocate that much even for the first attempt. The only exception to that is the call in key_event() in unix/window.c, which uses a fixed-size output buffer, but its input will always be the data generated by an X keystroke event. So that one can only overrun the buffer if an X key event manages to translate into more than 32 wide characters of text - and even if that does come up in some exotic edge case, it will at least not be happening under _enemy_ control. --- unix/unicode.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/unix/unicode.c b/unix/unicode.c index c1d76a42..4eaa45f4 100644 --- a/unix/unicode.c +++ b/unix/unicode.c @@ -31,6 +31,8 @@ int mb_to_wc(int codepage, int flags, const char *mbstr, int mblen, memset(&state, 0, sizeof state); while (mblen > 0) { + if (n >= wclen) + return n; size_t i = mbrtowc(wcstr+n, mbstr, (size_t)mblen, &state); if (i == (size_t)-1 || i == (size_t)-2) break; @@ -44,6 +46,8 @@ int mb_to_wc(int codepage, int flags, const char *mbstr, int mblen, int n = 0; while (mblen > 0) { + if (n >= wclen) + return n; wcstr[n] = 0xD800 | (mbstr[0] & 0xFF); n++; mbstr++; From fe00a2928c5f490be2d8f70a03af5529dfbaad7f Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 11 Mar 2022 19:11:35 +0000 Subject: [PATCH 05/16] Windows: diagnose failure to create the terminal window. It only fails in very unusual circumstances, but that's all the more reason to give a useful report when it does! --- windows/window.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/windows/window.c b/windows/window.c index c1cd0c4c..39a7ca34 100644 --- a/windows/window.c +++ b/windows/window.c @@ -575,6 +575,10 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) wgs.term_hwnd = CreateWindowExW( exwinmode, uappname, uappname, winmode, CW_USEDEFAULT, CW_USEDEFAULT, guess_width, guess_height, NULL, NULL, inst, NULL); + if (!wgs.term_hwnd) { + modalfatalbox("Unable to create terminal window: %s", + win_strerror(GetLastError())); + } memset(&dpi_info, 0, sizeof(struct _dpi_info)); init_dpi_info(); sfree(uappname); From 901667280ae2708df3a73618f85a03af15a8dcd6 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 14:33:01 +0000 Subject: [PATCH 06/16] Windows: initialise window_name and icon_name. It turns out that they're still NULL at the first moment that a SetWindowText call tries to read one of them, because they weren't initialised at startup! Apparently SetWindowText notices that it's been passed a null pointer, and does nothing in preference to failing, but it's still not what I _meant_ to do. --- windows/window.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/windows/window.c b/windows/window.c index 39a7ca34..1bc12fd7 100644 --- a/windows/window.c +++ b/windows/window.c @@ -563,6 +563,8 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) if (vt && vt->flags & BACKEND_RESIZE_FORBIDDEN) resize_forbidden = true; wchar_t *uappname = dup_mb_to_wc(DEFAULT_CODEPAGE, 0, appname); + window_name = dup_mb_to_wc(DEFAULT_CODEPAGE, 0, appname); + icon_name = dup_mb_to_wc(DEFAULT_CODEPAGE, 0, appname); if (!conf_get_bool(conf, CONF_scrollbar)) winmode &= ~(WS_VSCROLL); if (conf_get_int(conf, CONF_resize_action) == RESIZE_DISABLED || From 5de1df1b947076f3b9d2b8253a55aa93403c4ee2 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 17:36:54 +0000 Subject: [PATCH 07/16] Windows: avoid idempotent window title changes. By testing on a platform slow enough to show the flicker, I happened to notice that if your shell prompt resets the window title every time it's displayed, this was actually resulting in a call to SetWindowText every time, which caused the GUI to actually do work. There's certainly no need for that! We can at least avoid bothering if the new title is identical to the old one. --- windows/window.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/windows/window.c b/windows/window.c index 1bc12fd7..5a350b97 100644 --- a/windows/window.c +++ b/windows/window.c @@ -4758,16 +4758,26 @@ static int TranslateKey(UINT message, WPARAM wParam, LPARAM lParam, static void wintw_set_title(TermWin *tw, const char *title, int codepage) { + wchar_t *new_window_name = dup_mb_to_wc(codepage, 0, title); + if (!wcscmp(new_window_name, window_name)) { + sfree(new_window_name); + return; + } sfree(window_name); - window_name = dup_mb_to_wc(codepage, 0, title); + window_name = new_window_name; if (conf_get_bool(conf, CONF_win_name_always) || !IsIconic(wgs.term_hwnd)) SetWindowTextW(wgs.term_hwnd, window_name); } static void wintw_set_icon_title(TermWin *tw, const char *title, int codepage) { + wchar_t *new_icon_name = dup_mb_to_wc(codepage, 0, title); + if (!wcscmp(new_icon_name, icon_name)) { + sfree(new_icon_name); + return; + } sfree(icon_name); - icon_name = dup_mb_to_wc(codepage, 0, title); + icon_name = new_icon_name; if (!conf_get_bool(conf, CONF_win_name_always) && IsIconic(wgs.term_hwnd)) SetWindowTextW(wgs.term_hwnd, icon_name); } From 26dcfcbd4470eef31f61b6a617f3386947c4fad6 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 15:21:18 +0000 Subject: [PATCH 08/16] Make init_winver() idempotent. This way, anyone who needs to use the version data can quickly call init_winver to make sure it's been set up, and not waste too much faff redoing the actual work. --- windows/utils/version.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/windows/utils/version.c b/windows/utils/version.c index a53c2ad3..b626710e 100644 --- a/windows/utils/version.c +++ b/windows/utils/version.c @@ -4,6 +4,11 @@ DWORD osMajorVersion, osMinorVersion, osPlatformId; void init_winver(void) { + static bool initialised = false; + if (initialised) + return; + initialised = true; + OSVERSIONINFO osVersion; static HMODULE kernel32_module; DECL_WINDOWS_FUNCTION(static, BOOL, GetVersionExA, (LPOSVERSIONINFO)); From 51f0057b673ac948de497194f3d8a6fb0c8230da Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 17:10:11 +0000 Subject: [PATCH 09/16] Add a '#define LEGACY_WINDOWS'. This will be used to wrap some of the stranger workarounds we're keeping in this code base for the purposes of backwards compatibility to seriously old platforms like Win95. --- defs.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/defs.h b/defs.h index 3c30b6b6..354c208f 100644 --- a/defs.h +++ b/defs.h @@ -42,6 +42,8 @@ #define SIZEx "Ix" #define SIZEu "Iu" uintmax_t strtoumax(const char *nptr, char **endptr, int base); +/* Also, define a LEGACY_WINDOWS flag to enable other workarounds */ +#define LEGACY_WINDOWS #else #include /* Because we still support older MSVC libraries which don't recognise the From 83ff08f9db58e516f81e948e8256b50940145d30 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 11 Mar 2022 18:07:50 +0000 Subject: [PATCH 10/16] Remove hard dependency on GetFileAttributesEx. This fixes a load-time failure on versions of Windows too old to have that function in kernel32.dll. We use it to determine whether a file was safe to overwrite in the context of PuTTY session logging: if it's safe, we skip the 'do you want to overwrite or append?' dialog box. On earlier Windows you can use FindFirstFile to get a similar effect, so that's what we fall back to. It's not quite the same, though - if you pass a wildcard then it will succeed when you'd rather it had failed. But it's good enough to at least work in normal cases. --- .../utils/open_for_write_would_lose_data.c | 66 +++++++++++++++---- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/windows/utils/open_for_write_would_lose_data.c b/windows/utils/open_for_write_would_lose_data.c index 3891f51e..2aef5c5a 100644 --- a/windows/utils/open_for_write_would_lose_data.c +++ b/windows/utils/open_for_write_would_lose_data.c @@ -4,20 +4,19 @@ #include "putty.h" -bool open_for_write_would_lose_data(const Filename *fn) +/* + * This is slightly fiddly because we want to be backwards-compatible + * with systems too old to have GetFileAttributesEx. The next best + * thing is FindFirstFile, which will return a different data + * structure, but one that also contains the fields we want. (But it + * will behave more unhelpfully - for this application - in the + * presence of wildcards, so we'd prefer to use GFAE if we can.) + */ + +static inline bool open_for_write_would_lose_data_impl( + DWORD dwFileAttributes, DWORD nFileSizeHigh, DWORD nFileSizeLow) { - WIN32_FILE_ATTRIBUTE_DATA attrs; - if (!GetFileAttributesEx(fn->path, GetFileExInfoStandard, &attrs)) { - /* - * Generally, if we don't identify a specific reason why we - * should return true from this function, we return false, and - * let the subsequent attempt to open the file for real give a - * more useful error message. - */ - return false; - } - if (attrs.dwFileAttributes & (FILE_ATTRIBUTE_DEVICE | - FILE_ATTRIBUTE_DIRECTORY)) { + if (dwFileAttributes & (FILE_ATTRIBUTE_DEVICE|FILE_ATTRIBUTE_DIRECTORY)) { /* * File is something other than an ordinary disk file, so * opening it for writing will not cause truncation. (It may @@ -25,7 +24,7 @@ bool open_for_write_would_lose_data(const Filename *fn) */ return false; } - if (attrs.nFileSizeHigh == 0 && attrs.nFileSizeLow == 0) { + if (nFileSizeHigh == 0 && nFileSizeLow == 0) { /* * File is zero-length (or may be a named pipe, which * dwFileAttributes can't tell apart from a regular file), so @@ -36,3 +35,42 @@ bool open_for_write_would_lose_data(const Filename *fn) } return true; } + +bool open_for_write_would_lose_data(const Filename *fn) +{ + static HMODULE kernel32_module; + DECL_WINDOWS_FUNCTION(static, BOOL, GetFileAttributesExA, + (LPCSTR, GET_FILEEX_INFO_LEVELS, LPVOID)); + + if (!kernel32_module) { + kernel32_module = load_system32_dll("kernel32.dll"); + GET_WINDOWS_FUNCTION(kernel32_module, GetFileAttributesExA); + } + + if (p_GetFileAttributesExA) { + WIN32_FILE_ATTRIBUTE_DATA attrs; + if (!p_GetFileAttributesExA(fn->path, GetFileExInfoStandard, &attrs)) { + /* + * Generally, if we don't identify a specific reason why we + * should return true from this function, we return false, and + * let the subsequent attempt to open the file for real give a + * more useful error message. + */ + return false; + } + return open_for_write_would_lose_data_impl( + attrs.dwFileAttributes, attrs.nFileSizeHigh, attrs.nFileSizeLow); + } else { + WIN32_FIND_DATA fd; + HANDLE h = FindFirstFile(fn->path, &fd); + if (h == INVALID_HANDLE_VALUE) { + /* + * As above, if we can't find the file at all, return false. + */ + return false; + } + CloseHandle(h); + return open_for_write_would_lose_data_impl( + fd.dwFileAttributes, fd.nFileSizeHigh, fd.nFileSizeLow); + } +} From 01c007121aa3baec3e02842c03171ec9a99882ae Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 11 Mar 2022 18:32:26 +0000 Subject: [PATCH 11/16] Remove hard dependencies on multi-monitor functions. These are more functions that don't exist on all our supported legacy versions of Windows, so we need to follow the same policy as everywhere else, by trying to acquire them at run time and having a fallback if they aren't available. --- windows/window.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/windows/window.c b/windows/window.c index 5a350b97..e8b99b93 100644 --- a/windows/window.c +++ b/windows/window.c @@ -197,6 +197,9 @@ bool tried_pal = false; COLORREF colorref_modifier = 0; enum MONITOR_DPI_TYPE { MDT_EFFECTIVE_DPI, MDT_ANGULAR_DPI, MDT_RAW_DPI, MDT_DEFAULT }; +DECL_WINDOWS_FUNCTION(static, BOOL, GetMonitorInfoA, (HMONITOR, LPMONITORINFO)); +DECL_WINDOWS_FUNCTION(static, HMONITOR, MonitorFromPoint, (POINT, DWORD)); +DECL_WINDOWS_FUNCTION(static, HMONITOR, MonitorFromWindow, (HWND, DWORD)); DECL_WINDOWS_FUNCTION(static, HRESULT, GetDpiForMonitor, (HMONITOR hmonitor, enum MONITOR_DPI_TYPE dpiType, UINT *dpiX, UINT *dpiY)); DECL_WINDOWS_FUNCTION(static, HRESULT, GetSystemMetricsForDpi, (int nIndex, UINT dpi)); DECL_WINDOWS_FUNCTION(static, HRESULT, AdjustWindowRectExForDpi, (LPRECT lpRect, DWORD dwStyle, BOOL bMenu, DWORD dwExStyle, UINT dpi)); @@ -1293,9 +1296,9 @@ static int get_font_width(HDC hdc, const TEXTMETRIC *tm) static void init_dpi_info(void) { if (dpi_info.cur_dpi.x == 0 || dpi_info.cur_dpi.y == 0) { - if (p_GetDpiForMonitor) { + if (p_GetDpiForMonitor && p_MonitorFromWindow) { UINT dpiX, dpiY; - HMONITOR currentMonitor = MonitorFromWindow( + HMONITOR currentMonitor = p_MonitorFromWindow( wgs.term_hwnd, MONITOR_DEFAULTTOPRIMARY); if (p_GetDpiForMonitor(currentMonitor, MDT_EFFECTIVE_DPI, &dpiX, &dpiY) == S_OK) { @@ -2618,27 +2621,26 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, GetCursorPos(&pt); #ifndef NO_MULTIMON - { + if (p_GetMonitorInfoA && p_MonitorFromPoint) { HMONITOR mon; MONITORINFO mi; - mon = MonitorFromPoint(pt, MONITOR_DEFAULTTONULL); + mon = p_MonitorFromPoint(pt, MONITOR_DEFAULTTONULL); if (mon != NULL) { mi.cbSize = sizeof(MONITORINFO); - GetMonitorInfo(mon, &mi); + p_GetMonitorInfoA(mon, &mi); if (mi.rcMonitor.left == pt.x && mi.rcMonitor.top == pt.y) { mouse_on_hotspot = true; } } - } -#else + } else +#endif if (pt.x == 0 && pt.y == 0) { mouse_on_hotspot = true; } -#endif if (is_full_screen() && press && button == MBT_LEFT && mouse_on_hotspot) { SendMessage(hwnd, WM_SYSCOMMAND, SC_MOUSEMENU, @@ -4075,6 +4077,9 @@ static void init_winfuncs(void) GET_WINDOWS_FUNCTION(user32_module, FlashWindowEx); GET_WINDOWS_FUNCTION(user32_module, ToUnicodeEx); GET_WINDOWS_FUNCTION_PP(winmm_module, PlaySound); + GET_WINDOWS_FUNCTION_NO_TYPECHECK(user32_module, GetMonitorInfoA); + GET_WINDOWS_FUNCTION_NO_TYPECHECK(user32_module, MonitorFromPoint); + GET_WINDOWS_FUNCTION_NO_TYPECHECK(user32_module, MonitorFromWindow); GET_WINDOWS_FUNCTION_NO_TYPECHECK(shcore_module, GetDpiForMonitor); GET_WINDOWS_FUNCTION_NO_TYPECHECK(user32_module, GetSystemMetricsForDpi); GET_WINDOWS_FUNCTION_NO_TYPECHECK(user32_module, AdjustWindowRectExForDpi); @@ -5685,23 +5690,24 @@ static bool is_full_screen() static bool get_fullscreen_rect(RECT * ss) { #if defined(MONITOR_DEFAULTTONEAREST) && !defined(NO_MULTIMON) + if (p_GetMonitorInfoA && p_MonitorFromWindow) { HMONITOR mon; MONITORINFO mi; - mon = MonitorFromWindow(wgs.term_hwnd, MONITOR_DEFAULTTONEAREST); + mon = p_MonitorFromWindow(wgs.term_hwnd, MONITOR_DEFAULTTONEAREST); mi.cbSize = sizeof(mi); - GetMonitorInfo(mon, &mi); + p_GetMonitorInfoA(mon, &mi); /* structure copy */ *ss = mi.rcMonitor; return true; -#else + } +#endif /* could also use code like this: ss->left = ss->top = 0; ss->right = GetSystemMetrics(SM_CXSCREEN); ss->bottom = GetSystemMetrics(SM_CYSCREEN); */ return GetClientRect(GetDesktopWindow(), ss); -#endif } From f500d24a95d1eac73cdde701b4849bf6a936e291 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 06:43:59 +0000 Subject: [PATCH 12/16] Windows: runtime switch between Unicode and ANSI windows. Turns out that PuTTY hasn't run successfully on legacy Windows since 0.66, in spite of an ongoing intention to keep it working. Among the reasons for this is that CreateWindowExW simply fails with ERROR_CALL_NOT_IMPLEMENTED: apparently Win95 and its ilk just didn't have fully-Unicode windows as an option. Fixed by resurrecting the previous code from the git history (in particular, code removed by commit 67e5ceb9a8e6bc2 was useful), and including it as a runtime alternative. One subtlety was that I found I had to name the A and W window classes differently (by appending ".ansi" to the A one): apparently they occupy the same namespace even though the names are in different character sets, so if you somehow manage to register both classes, they'll collide with each other without that tweak. --- windows/window.c | 147 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 116 insertions(+), 31 deletions(-) diff --git a/windows/window.c b/windows/window.c index e8b99b93..c81755f2 100644 --- a/windows/window.c +++ b/windows/window.c @@ -450,6 +450,72 @@ static void close_session(void *ignored_context) } } +/* + * Some machinery to deal with switching the window type between ANSI + * and Unicode. We prefer Unicode, but some PuTTY builds still try to + * run on machines so old that they don't support that mode. So we're + * prepared to fall back to an ANSI window if we have to. For this + * purpose, we swap out a few Windows API functions, and wrap + * SetWindowText so that if we're not in Unicode mode we first convert + * the wide string we're given. + */ +static bool unicode_window; +static BOOL (WINAPI *sw_PeekMessage)(LPMSG, HWND, UINT, UINT, UINT); +static LRESULT (WINAPI *sw_DispatchMessage)(const MSG *); +static LRESULT (WINAPI *sw_DefWindowProc)(HWND, UINT, WPARAM, LPARAM); +static void sw_SetWindowText(HWND hwnd, wchar_t *text) +{ + if (unicode_window) { + SetWindowTextW(hwnd, text); + } else { + char *mb = dup_wc_to_mb(DEFAULT_CODEPAGE, 0, text, "?", &ucsdata); + SetWindowTextA(hwnd, mb); + sfree(mb); + } +} + +static HINSTANCE hprev; + +/* + * Also, registering window classes has to be done in a fiddly way. + */ +#define SETUP_WNDCLASS(wndclass, classname) do { \ + wndclass.style = 0; \ + wndclass.lpfnWndProc = WndProc; \ + wndclass.cbClsExtra = 0; \ + wndclass.cbWndExtra = 0; \ + wndclass.hInstance = hinst; \ + wndclass.hIcon = LoadIcon(hinst, MAKEINTRESOURCE(IDI_MAINICON)); \ + wndclass.hCursor = LoadCursor(NULL, IDC_IBEAM); \ + wndclass.hbrBackground = NULL; \ + wndclass.lpszMenuName = NULL; \ + wndclass.lpszClassName = classname; \ + } while (0) +wchar_t *terminal_window_class_w(void) +{ + static wchar_t *classname = NULL; + if (!classname) + classname = dup_mb_to_wc(DEFAULT_CODEPAGE, 0, appname); + if (!hprev) { + WNDCLASSW wndclassw; + SETUP_WNDCLASS(wndclassw, classname); + RegisterClassW(&wndclassw); + } + return classname; +} +char *terminal_window_class_a(void) +{ + static char *classname = NULL; + if (!classname) + classname = dupcat(appname, ".ansi"); + if (!hprev) { + WNDCLASSA wndclassa; + SETUP_WNDCLASS(wndclassa, classname); + RegisterClassA(&wndclassa); + } + return classname; +} + const unsigned cmdline_tooltype = TOOLTYPE_HOST_ARG | TOOLTYPE_PORT_ARG | @@ -466,6 +532,7 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) dll_hijacking_protection(); hinst = inst; + hprev = prev; sk_init(); @@ -514,23 +581,6 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) */ gui_term_process_cmdline(conf, cmdline); - if (!prev) { - WNDCLASSW wndclass; - - wndclass.style = 0; - wndclass.lpfnWndProc = WndProc; - wndclass.cbClsExtra = 0; - wndclass.cbWndExtra = 0; - wndclass.hInstance = inst; - wndclass.hIcon = LoadIcon(inst, MAKEINTRESOURCE(IDI_MAINICON)); - wndclass.hCursor = LoadCursor(NULL, IDC_IBEAM); - wndclass.hbrBackground = NULL; - wndclass.lpszMenuName = NULL; - wndclass.lpszClassName = dup_mb_to_wc(DEFAULT_CODEPAGE, 0, appname); - - RegisterClassW(&wndclass); - } - memset(&ucsdata, 0, sizeof(ucsdata)); conf_cache_data(); @@ -577,9 +627,38 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) exwinmode |= WS_EX_TOPMOST; if (conf_get_bool(conf, CONF_sunken_edge)) exwinmode |= WS_EX_CLIENTEDGE; + +#ifdef TEST_ANSI_WINDOW + /* For developer testing of ANSI window support, pretend + * CreateWindowExW failed */ + wgs.term_hwnd = NULL; + SetLastError(ERROR_CALL_NOT_IMPLEMENTED); +#else + unicode_window = true; + sw_PeekMessage = PeekMessageW; + sw_DispatchMessage = DispatchMessageW; + sw_DefWindowProc = DefWindowProcW; wgs.term_hwnd = CreateWindowExW( - exwinmode, uappname, uappname, winmode, CW_USEDEFAULT, - CW_USEDEFAULT, guess_width, guess_height, NULL, NULL, inst, NULL); + exwinmode, terminal_window_class_w(), uappname, + winmode, CW_USEDEFAULT, CW_USEDEFAULT, + guess_width, guess_height, NULL, NULL, inst, NULL); +#endif + +#if defined LEGACY_WINDOWS || defined TEST_ANSI_WINDOW + if (!wgs.term_hwnd && GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) { + /* Fall back to an ANSI window, swapping in all the ANSI + * window message handling functions */ + unicode_window = false; + sw_PeekMessage = PeekMessageA; + sw_DispatchMessage = DispatchMessageA; + sw_DefWindowProc = DefWindowProcA; + wgs.term_hwnd = CreateWindowExA( + exwinmode, terminal_window_class_a(), appname, + winmode, CW_USEDEFAULT, CW_USEDEFAULT, + guess_width, guess_height, NULL, NULL, inst, NULL); + } +#endif + if (!wgs.term_hwnd) { modalfatalbox("Unable to create terminal window: %s", win_strerror(GetLastError())); @@ -776,13 +855,13 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) handle_wait_activate(hwl, n - WAIT_OBJECT_0); handle_wait_list_free(hwl); - while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) { + while (sw_PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) { if (msg.message == WM_QUIT) goto finished; /* two-level break */ HWND logbox = event_log_window(); if (!(IsWindow(logbox) && IsDialogMessage(logbox, &msg))) - DispatchMessageW(&msg); + sw_DispatchMessage(&msg); /* * WM_NETEVENT messages seem to jump ahead of others in @@ -2202,7 +2281,8 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, * within an interactive scrollbar-drag can be handled * differently. */ in_scrollbar_loop = true; - LRESULT result = DefWindowProcW(hwnd, message, wParam, lParam); + LRESULT result = sw_DefWindowProc( + hwnd, message, wParam, lParam); in_scrollbar_loop = false; return result; } @@ -2996,11 +3076,11 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, term, r.right - r.left, r.bottom - r.top); } if (wParam == SIZE_MINIMIZED) - SetWindowTextW(hwnd, - conf_get_bool(conf, CONF_win_name_always) ? - window_name : icon_name); + sw_SetWindowText(hwnd, + conf_get_bool(conf, CONF_win_name_always) ? + window_name : icon_name); if (wParam == SIZE_RESTORED || wParam == SIZE_MAXIMIZED) - SetWindowTextW(hwnd, window_name); + sw_SetWindowText(hwnd, window_name); if (wParam == SIZE_RESTORED) { processed_resize = false; clear_full_screen(); @@ -3222,7 +3302,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, } else { len = TranslateKey(message, wParam, lParam, buf); if (len == -1) - return DefWindowProcW(hwnd, message, wParam, lParam); + return sw_DefWindowProc(hwnd, message, wParam, lParam); if (len != 0) { /* @@ -3320,7 +3400,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, * post the things to us as part of a macro manoeuvre, * we're ready to cope. */ - { + if (unicode_window) { static wchar_t pending_surrogate = 0; wchar_t c = wParam; @@ -3334,6 +3414,11 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, } else if (!IS_SURROGATE(c)) { term_keyinputw(term, &c, 1); } + } else { + char c = (unsigned char)wParam; + term_seen_key_event(term); + if (ldisc) + term_keyinput(term, CP_ACP, &c, 1); } return 0; case WM_SYSCOLORCHANGE: @@ -3409,7 +3494,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, * Any messages we don't process completely above are passed through to * DefWindowProc() for default processing. */ - return DefWindowProcW(hwnd, message, wParam, lParam); + return sw_DefWindowProc(hwnd, message, wParam, lParam); } /* @@ -4771,7 +4856,7 @@ static void wintw_set_title(TermWin *tw, const char *title, int codepage) sfree(window_name); window_name = new_window_name; if (conf_get_bool(conf, CONF_win_name_always) || !IsIconic(wgs.term_hwnd)) - SetWindowTextW(wgs.term_hwnd, window_name); + sw_SetWindowText(wgs.term_hwnd, window_name); } static void wintw_set_icon_title(TermWin *tw, const char *title, int codepage) @@ -4784,7 +4869,7 @@ static void wintw_set_icon_title(TermWin *tw, const char *title, int codepage) sfree(icon_name); icon_name = new_icon_name; if (!conf_get_bool(conf, CONF_win_name_always) && IsIconic(wgs.term_hwnd)) - SetWindowTextW(wgs.term_hwnd, icon_name); + sw_SetWindowText(wgs.term_hwnd, icon_name); } static void wintw_set_scrollbar(TermWin *tw, int total, int start, int page) From a2b376af96072d6741caeb51b5f55fd05fd37157 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 15:02:12 +0000 Subject: [PATCH 13/16] Windows Pageant: turn 'has_security' into a global function. Now it can be called from places other than Pageant's WinMain(). In particular, the attempt to make a security descriptor in lock_interprocess_mutex() is gated on it. In return, however, I've tightened up the semantics. In normal PuTTY builds that aren't trying to support pre-NT systems, the function *unconditionally* returns true, on the grounds that we don't expect to target any system that doesn't support the security APIs, and if someone manages to contrive one anyway - or, more likely, if we some day introduce a bug in our loading of the security API functions - then this safety catch should make Pageant less likely to accidentally fall back to 'never mind, just run in insecure mode'. --- windows/pageant.c | 70 ++++++++++++++---------------- windows/platform.h | 1 + windows/utils/interprocess_mutex.c | 4 +- windows/utils/security.c | 14 ++++++ 4 files changed, 49 insertions(+), 40 deletions(-) diff --git a/windows/pageant.c b/windows/pageant.c index 0e3868a0..a08659e5 100644 --- a/windows/pageant.c +++ b/windows/pageant.c @@ -91,8 +91,6 @@ void modalfatalbox(const char *fmt, ...) exit(1); } -static bool has_security; - struct PassphraseProcStruct { bool modal; const char *help_topic; @@ -999,7 +997,7 @@ static char *answer_filemapping_message(const char *mapname) debug("maphandle = %p\n", maphandle); #endif - if (has_security) { + if (should_have_security()) { DWORD retd; if ((expectedsid = get_user_sid()) == NULL) { @@ -1405,14 +1403,7 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) hinst = inst; - /* - * Determine whether we're an NT system (should have security - * APIs) or a non-NT system (don't do security). - */ - init_winver(); - has_security = (osPlatformId == VER_PLATFORM_WIN32_NT); - - if (has_security) { + if (should_have_security()) { /* * Attempt to get the security API we need. */ @@ -1543,39 +1534,42 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) /* * Set up a named-pipe listener. */ - Plug *pl_plug; wpc->plc.vt = &winpgnt_vtable; wpc->plc.suppress_logging = true; - struct pageant_listen_state *pl = - pageant_listener_new(&pl_plug, &wpc->plc); - char *pipename = agent_named_pipe_name(); - Socket *sock = new_named_pipe_listener(pipename, pl_plug); - if (sk_socket_error(sock)) { - char *err = dupprintf("Unable to open named pipe at %s " - "for SSH agent:\n%s", pipename, - sk_socket_error(sock)); - MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK); - return 1; - } - pageant_listener_got_socket(pl, sock); - - /* - * If we've been asked to write out an OpenSSH config file - * pointing at the named pipe, do so. - */ - if (openssh_config_file) { - FILE *fp = fopen(openssh_config_file, "w"); - if (!fp) { - char *err = dupprintf("Unable to write OpenSSH config file " - "to %s", openssh_config_file); + if (should_have_security()) { + Plug *pl_plug; + struct pageant_listen_state *pl = + pageant_listener_new(&pl_plug, &wpc->plc); + char *pipename = agent_named_pipe_name(); + Socket *sock = new_named_pipe_listener(pipename, pl_plug); + if (sk_socket_error(sock)) { + char *err = dupprintf("Unable to open named pipe at %s " + "for SSH agent:\n%s", pipename, + sk_socket_error(sock)); MessageBox(NULL, err, "Pageant Error", MB_ICONERROR | MB_OK); return 1; } - fprintf(fp, "IdentityAgent %s\n", pipename); - fclose(fp); - } + pageant_listener_got_socket(pl, sock); - sfree(pipename); + /* + * If we've been asked to write out an OpenSSH config file + * pointing at the named pipe, do so. + */ + if (openssh_config_file) { + FILE *fp = fopen(openssh_config_file, "w"); + if (!fp) { + char *err = dupprintf("Unable to write OpenSSH config " + "file to %s", openssh_config_file); + MessageBox(NULL, err, "Pageant Error", + MB_ICONERROR | MB_OK); + return 1; + } + fprintf(fp, "IdentityAgent %s\n", pipename); + fclose(fp); + } + + sfree(pipename); + } /* * Set up the window class for the hidden window that receives diff --git a/windows/platform.h b/windows/platform.h index d15e1a68..5264e8f0 100644 --- a/windows/platform.h +++ b/windows/platform.h @@ -574,6 +574,7 @@ void dll_hijacking_protection(void); const char *get_system_dir(void); HMODULE load_system32_dll(const char *libname); const char *win_strerror(int error); +bool should_have_security(void); void restrict_process_acl(void); bool restricted_acl(void); void escape_registry_key(const char *in, strbuf *out); diff --git a/windows/utils/interprocess_mutex.c b/windows/utils/interprocess_mutex.c index a22ec820..8612a298 100644 --- a/windows/utils/interprocess_mutex.c +++ b/windows/utils/interprocess_mutex.c @@ -13,8 +13,8 @@ HANDLE lock_interprocess_mutex(const char *mutexname, char **error) PACL acl = NULL; HANDLE mutex = NULL; - if (!make_private_security_descriptor(MUTEX_ALL_ACCESS, - &psd, &acl, error)) + if (should_have_security() && !make_private_security_descriptor( + MUTEX_ALL_ACCESS, &psd, &acl, error)) goto out; SECURITY_ATTRIBUTES sa; diff --git a/windows/utils/security.c b/windows/utils/security.c index 2d899b83..1a4770b6 100644 --- a/windows/utils/security.c +++ b/windows/utils/security.c @@ -20,6 +20,20 @@ DEF_WINDOWS_FUNCTION(GetSecurityInfo); DEF_WINDOWS_FUNCTION(SetSecurityInfo); DEF_WINDOWS_FUNCTION(SetEntriesInAclA); +bool should_have_security(void) +{ +#ifdef LEGACY_WINDOWS + /* Legacy pre-NT platforms are not expected to have any of these APIs */ + init_winver(); + return (osPlatformId == VER_PLATFORM_WIN32_NT); +#else + /* In the up-to-date PuTTY builds which do not support those + * platforms, unconditionally return true, to minimise the risk of + * compiling out security checks. */ + return true; +#endif +} + bool got_advapi(void) { static bool attempted = false; From 3f76a86c13a315a8b657642987935d72f86707c6 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 15:18:38 +0000 Subject: [PATCH 14/16] Windows Pageant: deal with PeekMessageW failing on legacy Windows. This makes Pageant run on Win95 again. Previously (after fixing the startup-time failures due to missing security APIs) it would go into an uninterruptible CPU-consuming spin in the message loop when every attempt to retrieve its messages failed because PeekMessageW doesn't work at all on the 95 series. --- windows/pageant.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/windows/pageant.c b/windows/pageant.c index a08659e5..5ba16801 100644 --- a/windows/pageant.c +++ b/windows/pageant.c @@ -1382,6 +1382,23 @@ static NORETURN void opt_error(const char *fmt, ...) exit(1); } +#ifdef LEGACY_WINDOWS +BOOL sw_PeekMessage(LPMSG msg, HWND hwnd, UINT min, UINT max, UINT remove) +{ + static bool unicode_unavailable = false; + if (!unicode_unavailable) { + BOOL ret = PeekMessageW(msg, hwnd, min, max, remove); + if (!ret && GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) + unicode_unavailable = true; /* don't try again */ + else + return ret; + } + return PeekMessageA(msg, hwnd, min, max, remove); +} +#else +#define sw_PeekMessage PeekMessageW +#endif + int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) { MSG msg; @@ -1732,7 +1749,7 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) handle_wait_activate(hwl, n - WAIT_OBJECT_0); handle_wait_list_free(hwl); - while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) { + while (sw_PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) { if (msg.message == WM_QUIT) goto finished; /* two-level break */ From f23a84cf7c89cc4918976178c19abb654f76a7ef Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 16:16:01 +0000 Subject: [PATCH 15/16] windows/unicode.c: manually speak UTF-8. This is another fallback needed on Win95, where the Win32 API functions to convert between multibyte and wide strings exist, but they haven't heard of the UTF-8 code page. PuTTY can't really do without that these days. (In particular, if a server sends a remote window-title escape sequence while the terminal is in UTF-8 mode, then _something_ needs to translate the UTF-8 data into Unicode for Windows to reconvert into the character set used in window titles.) This is a weird enough thing to be doing that I've put it under the new #ifdef LEGACY_WINDOWS, so behaviour in the standard builds should be unchanged. --- windows/unicode.c | 85 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 81 insertions(+), 4 deletions(-) diff --git a/windows/unicode.c b/windows/unicode.c index 9ffff5e9..943c3c2d 100644 --- a/windows/unicode.c +++ b/windows/unicode.c @@ -1195,16 +1195,93 @@ int wc_to_mb(int codepage, int flags, const wchar_t *wcstr, int wclen, } return p - mbstr; } else { - int defused; - return WideCharToMultiByte(codepage, flags, wcstr, wclen, - mbstr, mblen, defchr, &defused); + int defused, ret; + ret = WideCharToMultiByte(codepage, flags, wcstr, wclen, + mbstr, mblen, defchr, &defused); + if (ret) + return ret; + +#ifdef LEGACY_WINDOWS + /* + * Fallback for legacy platforms too old to support UTF-8: if + * the codepage is UTF-8, we can do the translation ourselves. + */ + if (codepage == CP_UTF8 && mblen > 0 && wclen > 0) { + size_t remaining = mblen; + char *p = mbstr; + + while (wclen > 0) { + unsigned long wc = (wclen--, *wcstr++); + if (wclen > 0 && IS_SURROGATE_PAIR(wc, *wcstr)) { + wc = FROM_SURROGATES(wc, *wcstr); + wclen--, wcstr++; + } + + char utfbuf[6]; + size_t utflen = encode_utf8(utfbuf, wc); + if (utflen <= remaining) { + memcpy(p, utfbuf, utflen); + p += utflen; + remaining -= utflen; + } else { + return p - mbstr; + } + } + + return p - mbstr; + } +#endif + + /* No other fallbacks are available */ + return 0; } } int mb_to_wc(int codepage, int flags, const char *mbstr, int mblen, wchar_t *wcstr, int wclen) { - return MultiByteToWideChar(codepage, flags, mbstr, mblen, wcstr, wclen); + int ret = MultiByteToWideChar(codepage, flags, mbstr, mblen, wcstr, wclen); + if (ret) + return ret; + +#ifdef LEGACY_WINDOWS + /* + * Fallback for legacy platforms too old to support UTF-8: if the + * codepage is UTF-8, we can do the translation ourselves. + */ + if (codepage == CP_UTF8 && mblen > 0 && wclen > 0) { + size_t remaining = wclen; + wchar_t *p = wcstr; + + while (mblen > 0) { + char utfbuf[7]; + int thissize = mblen < 6 ? mblen : 6; + memcpy(utfbuf, mbstr, thissize); + utfbuf[thissize] = '\0'; + + const char *utfptr = utfbuf; + wchar_t wcbuf[2]; + size_t nwc = decode_utf8_to_wchar(&utfptr, wcbuf); + + for (size_t i = 0; i < nwc; i++) { + if (remaining > 0) { + remaining--; + *p++ = wcbuf[i]; + } else { + return p - wcstr; + } + } + + mbstr += (utfptr - utfbuf); + mblen -= (utfptr - utfbuf); + } + + return p - wcstr; + } +#endif + + /* No other fallbacks are available */ + return 0; } bool is_dbcs_leadbyte(int codepage, char byte) From 5d58931b51ff1223f1b2208956538cfadd702c0b Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 12 Mar 2022 20:17:30 +0000 Subject: [PATCH 16/16] Fix trust status when Interactor returns a seat. While testing the unrelated pile of commits just past, I accidentally started a Cygwin saved session I hadn't run in ages which used the old Telnet-based cygtermd as a local proxy command, and found that it presented the Cygwin prompt with a trust sigil. Oops! It turns out that this is because interactor_return_seat does two things that can change the real seat's trust status, and it does them in the wrong order: it defaults the status back to trusted (as if the seat was brand new, because that's how they start out), and it calls tempseat_flush which may have buffered a trust-status reset while the seat was borrowed. The former should not override the latter! --- proxy/interactor.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/proxy/interactor.c b/proxy/interactor.c index 958e5a98..d069d226 100644 --- a/proxy/interactor.c +++ b/proxy/interactor.c @@ -47,8 +47,18 @@ void interactor_return_seat(Interactor *itr) if (!is_tempseat(tempseat)) return; /* no-op */ - tempseat_flush(tempseat); + /* + * We're about to hand this seat back to the parent Interactor to + * do its own thing with. It will typically expect to start in the + * same state as if the seat had never been borrowed, i.e. in the + * starting trust state. + * + * However, this may be overridden by the tempseat_flush call. + */ Seat *realseat = tempseat_get_real(tempseat); + seat_set_trust_status(realseat, true); + + tempseat_flush(tempseat); interactor_set_seat(itr, realseat); tempseat_free(tempseat); @@ -60,14 +70,6 @@ void interactor_return_seat(Interactor *itr) Interactor *itr_top = interactor_toplevel(itr, NULL); if (itr_top->last_to_talk) interactor_announce(itr); - - /* - * We're about to hand this seat back to the parent Interactor to - * do its own thing with. It will typically expect to start in the - * same state as if the seat had never been borrowed, i.e. in the - * starting trust state. - */ - seat_set_trust_status(realseat, true); } InteractionReadySeat interactor_announce(Interactor *itr)