- Sep 19, 2021
-
-
Dmytro D authored
-
- Mar 13, 2021
-
-
Simon Tatham authored
Now we pass the whole set of fingerprints, and also a displayable format for the full host public key. NFC: this commit doesn't modify any of the host key prompts to _use_ any of the new information. That's coming next.
-
Simon Tatham authored
verify_ssh_manual_host_key() now takes an array of all key fingerprints instead of just the default type, which means that an expected key fingerprint stored in the session configuration can now be matched against any of them.
-
- Mar 04, 2021
-
-
Simon Tatham authored
Thanks to a user for pointing out that these do exist, but aren't mentioned on the web page I've been citing as a reference so far, and moreover, they need to be identified in a slightly different way.
-
- Feb 13, 2021
-
-
Simon Tatham authored
I just re-checked the official list page and found that VS2019 has grown another five minor versions since commit 0a4e068a.
-
- Mar 10, 2020
-
-
Lars Brinkhoff authored
The motivation is for the SUPDUP protocol. The server may send a signal for the terminal to reset any input buffers. After this, the server will not know the state of the terminal, so it is required to send its cursor position back.
-
- Feb 09, 2020
-
-
Simon Tatham authored
An assortment of errors: int vs size_t confusion (probably undetected since the big switchover in commit 0cda34c6), some outright spurious parameters after the format string (copy-paste errors), a particularly silly one in pscp.c (a comma between two halves of what should have been a single string literal), and a _missing_ format string in ssh.c (but luckily in a context where the only text that would be wrongly treated as a format string was error messages generated elsewhere in PuTTY). (cherry picked from commit 247866a9)
-
Simon Tatham authored
The entry for 19.0 which we included in advance of its listing on the official page is now confirmed, and also three followup versions. (cherry picked from commit 0a4e068a)
-
Simon Tatham authored
UBsan pointed out another memcpy from NULL (again with length 0) in the prompts_t system. When I looked at it, I realised that firstly prompt_ensure_result_size was an early not-so-good implementation of sgrowarray_nm that would benefit from being replaced with a call to the real one, and secondly, the whole system for storing prompt results should really have been replaced with strbufs with the no-move option, because that's doing all the same jobs better. So, now each prompt_t holds a strbuf in place of its previous manually managed string. prompt_ensure_result_size is gone (the console prompt-reading functions use strbuf_append, and everything else just adds to the strbuf in the usual marshal.c way). New functions exist to retrieve a prompt_t's result, either by reference or copied. (cherry picked from commit cd6bc14f)
-
- Jan 30, 2020
-
-
Simon Tatham authored
This is simpler than FLAG_VERBOSE: everywhere we need to check it, we have a Seat available, so we can just make it a Seat query method.
-
Simon Tatham authored
The global 'int flags' has always been an ugly feature of this code base, and I suddenly thought that perhaps it's time to start throwing it out, one flag at a time, until it's totally unused. My first target is FLAG_VERBOSE. This was usually set by cmdline.c when it saw a -v option on the program's command line, except that GUI PuTTY itself sets it unconditionally on startup. And then various bits of the code would check it in order to decide whether to print a given message. In the current system of front-end abstraction traits, there's no _one_ place that I can move it to. But there are two: every place that checked FLAG_VERBOSE has access to either a Seat or a LogPolicy. So now each of those traits has a query method for 'do I want verbose messages?'. A good effect of this is that subsidiary Seats, like the ones used in Uppity for the main SSH server module itself and the server end of shell channels, now get to have their own verbosity setting instead of inheriting the one global one. In fact I don't expect any code using those Seats to be generating any messages at all, but if that changes later, we'll have a way to control it. (Who knows, perhaps logging in Uppity might become a thing.) As part of this cleanup, I've added a new flag to cmdline_tooltype, called TOOLTYPE_NO_VERBOSE_OPTION. The unconditionally-verbose tools now set that, and it has the effect of making cmdline.c disallow -v completely. So where 'putty -v' would previously have been silently ignored ("I was already verbose"), it's now an error, reminding you that that option doesn't actually do anything. Finally, the 'default_logpolicy' provided by uxcons.c and wincons.c (with identical definitions) has had to move into a new file of its own, because now it has to ask cmdline.c for the verbosity setting as well as asking console.c for the rest of its methods. So there's a new file clicons.c which can only be included by programs that link against both cmdline.c _and_ one of the *cons.c, and I've renamed the logpolicy to reflect that.
-
- Jan 26, 2020
-
-
Simon Tatham authored
An assortment of errors: int vs size_t confusion (probably undetected since the big switchover in commit 0cda34c6), some outright spurious parameters after the format string (copy-paste errors), a particularly silly one in pscp.c (a comma between two halves of what should have been a single string literal), and a _missing_ format string in ssh.c (but luckily in a context where the only text that would be wrongly treated as a format string was error messages generated elsewhere in PuTTY).
-
Simon Tatham authored
The entry for 19.0 which we included in advance of its listing on the official page is now confirmed, and also three followup versions.
-
- Jan 21, 2020
-
-
Simon Tatham authored
UBsan pointed out another memcpy from NULL (again with length 0) in the prompts_t system. When I looked at it, I realised that firstly prompt_ensure_result_size was an early not-so-good implementation of sgrowarray_nm that would benefit from being replaced with a call to the real one, and secondly, the whole system for storing prompt results should really have been replaced with strbufs with the no-move option, because that's doing all the same jobs better. So, now each prompt_t holds a strbuf in place of its previous manually managed string. prompt_ensure_result_size is gone (the console prompt-reading functions use strbuf_append, and everything else just adds to the strbuf in the usual marshal.c way). New functions exist to retrieve a prompt_t's result, either by reference or copied.
-
- Sep 08, 2019
-
-
Simon Tatham authored
The number of people has been steadily increasing who read our source code with an editor that thinks tab stops are 4 spaces apart, as opposed to the traditional tty-derived 8 that the PuTTY code expects. So I've been wondering for ages about just fixing it, and switching to a spaces-only policy throughout the code. And I recently found out about 'git blame -w', which should make this change not too disruptive for the purposes of source-control archaeology; so perhaps now is the time. While I'm at it, I've also taken the opportunity to remove all the trailing spaces from source lines (on the basis that git dislikes them, and is the only thing that seems to have a strong opinion one way or the other). Apologies to anyone downstream of this code who has complicated patch sets to rebase past this change. I don't intend it to be needed again.
-
- Mar 22, 2019
-
-
Simon Tatham authored
Thanks to Sean Kain for pointing out MS's web page listing all the known _MSC_VER values and their translations. To make it an easier and more mechanical process to update the list in future, I've completely replaced our previous text for each version with a straight paste of the exact string translations from that web page (plus Sean Kain's extra value for VS2019, which isn't listed on that page yet). That changes the exact wording of all the previous translations, mostly cosmetically (although it also fixes the version number for _MSC_VER=1912). Since many of the new translations end with a version number in parentheses, I've removed the parens around the following explicit statement of _MSC_VER, so they don't look silly next to each other.
-
- Mar 16, 2019
-
-
Simon Tatham authored
In terminal-based GUI applications, this is passed through to term_set_trust_status, to toggle whether lines are prefixed with the new trust sigil. In console applications, the function returns false, indicating to the backend that it should employ some other technique for spoofing protection.
-
Simon Tatham authored
With this change, we stop expecting to find putty.chm alongside the executable file. That was a security hazard comparable to DLL hijacking, because of the risk that a malicious CHM file could be dropped into the same directory as putty.exe (e.g. if someone ran PuTTY from their browser's download dir).. Instead, the standalone putty.exe (and other binaries needing help) embed the proper CHM file within themselves, as a Windows resource, and if called on to display the help then they write the file out to a temporary location. This has the advantage that if you download and run the standalone putty.exe then you actually _get_ help, which previously didn't happen! The versions of the binaries in the installer don't each contain a copy of the help file; that would be extravagant. Instead, the installer itself writes a registry entry pointing at the proper help file, and the executables will look there. Another effect of this commit is that I've withdrawn support for the older .HLP format completely. It's now entirely outdated, and supporting it through this security fix would have been a huge pain.
-
- Mar 09, 2019
-
-
Simon Tatham authored
Now instead of taking raw arguments to configure the output StripCtrlChars with, it takes an enumerated value giving the context of what's being sanitised, and allows the seat to decide what the output parameters for that context should be. The only context currently used is SIC_BANNER (SSH login banners). I've also added a not-yet-used one for keyboard-interactive prompts.
-
- Mar 06, 2019
-
-
Simon Tatham authored
If centralised code like the SSH implementation wants to sanitise escape sequences out of a piece of server-provided text, it will need to do it by making a locale-based StripCtrlChars if it's running in a console context, or a Terminal-based one if it's in a GUI terminal- window application. All the other changes of behaviour needed between those two contexts are handled by providing reconfigurable methods in the Seat vtable; this one is no different. So now there's a new method in the Seat vtable that will construct a StripCtrlChars appropriate to that kind of seat. Terminal-window seats (gtkwin.c, window.c) implement it by calling the new stripctrl_new_term(), and console ones use the locale- based stripctrl_new().
-
- Feb 28, 2019
-
-
Simon Tatham authored
The idea of these is that they centralise the common idiom along the lines of if (logical_array_len >= physical_array_size) { physical_array_size = logical_array_len * 5 / 4 + 256; array = sresize(array, physical_array_size, ElementType); } which happens at a zillion call sites throughout this code base, with different random choices of the geometric factor and additive constant, sometimes forgetting them completely, and generally doing a lot of repeated work. The new macro sgrowarray(array,size,n) has the semantics: here are the array pointer and its physical size for you to modify, now please ensure that the nth element exists, so I can write into it. And sgrowarrayn(array,size,n,m) is the same except that it ensures that the array has size at least n+m (so sgrowarray is just the special case where m=1). Now that this is a single centralised implementation that will be used everywhere, I've also gone to more effort in the implementation, with careful overflow checks that would have been painful to put at all the previous call sites. This commit also switches over every use of sresize(), apart from a few where I really didn't think it would gain anything. A consequence of that is that a lot of array-size variables have to have their types changed to size_t, because the macros require that (they address-take the size to pass to the underlying function).
-
- Feb 06, 2019
-
-
Simon Tatham authored
This is a general cleanup which has been overdue for some time: lots of length fields are now the machine word type rather than the (in practice) fixed 'int'.
-
- Jan 03, 2019
-
-
Simon Tatham authored
misc.c has always contained a combination of things that are tied tightly into the PuTTY code base (e.g. they use the conf system, or work with our sockets abstraction) and things that are pure standalone utility functions like nullstrcmp() which could quite happily be dropped into any C program without causing a link failure. Now the latter kind of standalone utility code lives in the new source file utils.c, whose only external dependency is on memory.c (for snew, sfree etc), which in turn requires the user to provide an out_of_memory() function. So it should now be much easier to link test programs that use PuTTY's low-level functions without also pulling in half its bulky infrastructure. In the process, I came across a memory allocation logging system enabled by -DMALLOC_LOG that looks long since bit-rotted; in any case we have much more advanced tools for that kind of thing these days, like valgrind and Leak Sanitiser, so I've just removed it rather than trying to transplant it somewhere sensible. (We can always pull it back out of the version control history if really necessary, but I haven't used it in at least a decade.) The other slightly silly thing I did was to give bufchain a function pointer field that points to queue_idempotent_callback(), and disallow direct setting of the 'ic' field in favour of calling bufchain_set_callback which will fill that pointer in too. That allows the bufchain system to live in utils.c rather than misc.c, so that programs can use it without also having to link in the callback system or provide an annoying stub of that function. In fact that's just allowed me to remove stubs of that kind from PuTTYgen and Pageant!
-
- Dec 01, 2018
-
-
Simon Tatham authored
The ad-hoc code that received data from the SCP or SFTP server predated even not-very-modern conveniences such as bufchain, and was quite horrible and cumbersome. Particularly nasty was the part where ssh_scp_recv set a _global_ pointer variable to the buffer it was in the middle of writing to, and then recursed and expected a callback to use that pointer. That caused clang-analyzer to grumble at me, in a particular case where the output buffer was in the ultimate caller's stack frame; even though I'm confident the code _worked_, I can't blame clang for being unhappy! So now we do things the modern and much simpler way: the callback when data comes in just puts it on a bufchain, and the top-level ssh_scp_recv repeatedly waits until data arrives in the bufchain and then copies it to the output buffer.
-
- Nov 27, 2018
-
-
Simon Tatham authored
Now they live in their own file memory.c. The advantage of this is that you can link them into a binary without also pulling in the rest of misc.c with its various dependencies on other parts of the code, such as conf.c.
-
- Nov 04, 2018
-
-
Pavel I. Kryukov authored
-
- Nov 03, 2018
-
-
Simon Tatham authored
This is another cleanup I felt a need for while I was doing boolification. If you define a function or variable in one .c file and declare it extern in another, then nothing will check you haven't got the types of the two declarations mismatched - so when you're _changing_ the type, it's a pain to make sure you've caught all the copies of it. It's better to put all those extern declarations in header files, so that the declaration in the header is also in scope for the definition. Then the compiler will complain if they don't match, which is what I want.
-
Simon Tatham authored
Notably toint(), which ought to compile down to the identity function in any case so you don't really want to put in a pointless call overhead, and make_ptrlen() (and a couple of its wrappers) which is standing in for what ought to be a struct-literal syntax.
-
Simon Tatham authored
My normal habit these days, in new code, is to treat int and bool as _almost_ completely separate types. I'm still willing to use C's implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine, no need to spell it out as blob.len != 0), but generally, if a variable is going to be conceptually a boolean, I like to declare it bool and assign to it using 'true' or 'false' rather than 0 or 1. PuTTY is an exception, because it predates the C99 bool, and I've stuck to its existing coding style even when adding new code to it. But it's been annoying me more and more, so now that I've decided C99 bool is an acceptable thing to require from our toolchain in the first place, here's a quite thorough trawl through the source doing 'boolification'. Many variables and function parameters are now typed as bool rather than int; many assignments of 0 or 1 to those variables are now spelled 'true' or 'false'. I managed this thorough conversion with the help of a custom clang plugin that I wrote to trawl the AST and apply heuristics to point out where things might want changing. So I've even managed to do a decent job on parts of the code I haven't looked at in years! To make the plugin's work easier, I pushed platform front ends generally in the direction of using standard 'bool' in preference to platform-specific boolean types like Windows BOOL or GTK's gboolean; I've left the platform booleans in places they _have_ to be for the platform APIs to work right, but variables only used by my own code have been converted wherever I found them. In a few places there are int values that look very like booleans in _most_ of the places they're used, but have a rarely-used third value, or a distinction between different nonzero values that most users don't care about. In these cases, I've _removed_ uses of 'true' and 'false' for the return values, to emphasise that there's something more subtle going on than a simple boolean answer: - the 'multisel' field in dialog.h's list box structure, for which the GTK front end in particular recognises a difference between 1 and 2 but nearly everything else treats as boolean - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you something about the specific location of the urgent pointer, but most clients only care about 0 vs 'something nonzero' - the return value of wc_match, where -1 indicates a syntax error in the wildcard. - the return values from SSH-1 RSA-key loading functions, which use -1 for 'wrong passphrase' and 0 for all other failures (so any caller which already knows it's not loading an _encrypted private_ key can treat them as boolean) - term->esc_query, and the 'query' parameter in toggle_mode in terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h, but can also hold -1 for some other intervening character that we don't support. In a few places there's an integer that I haven't turned into a bool even though it really _can_ only take values 0 or 1 (and, as above, tried to make the call sites consistent in not calling those values true and false), on the grounds that I thought it would make it more confusing to imply that the 0 value was in some sense 'negative' or bad and the 1 positive or good: - the return value of plug_accepting uses the POSIXish convention of 0=success and nonzero=error; I think if I made it bool then I'd also want to reverse its sense, and that's a job for a separate piece of work. - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1 represent the default and alternate screens. There's no obvious reason why one of those should be considered 'true' or 'positive' or 'success' - they're just indices - so I've left it as int. ssh_scp_recv had particularly confusing semantics for its previous int return value: its call sites used '<= 0' to check for error, but it never actually returned a negative number, just 0 or 1. Now the function and its call sites agree that it's a bool. In a couple of places I've renamed variables called 'ret', because I don't like that name any more - it's unclear whether it means the return value (in preparation) for the _containing_ function or the return value received from a subroutine call, and occasionally I've accidentally used the same variable for both and introduced a bug. So where one of those got in my way, I've renamed it to 'toret' or 'retd' (the latter short for 'returned') in line with my usual modern practice, but I haven't done a thorough job of finding all of them. Finally, one amusing side effect of doing this is that I've had to separate quite a few chained assignments. It used to be perfectly fine to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a the 'true' defined by stdbool.h, that idiom provokes a warning from gcc: 'suggest parentheses around assignment used as truth value'!
-
Simon Tatham authored
This commit includes <stdbool.h> from defs.h and deletes my traditional definitions of TRUE and FALSE, but other than that, it's a 100% mechanical search-and-replace transforming all uses of TRUE and FALSE into the C99-standardised lowercase spellings. No actual types are changed in this commit; that will come next. This is just getting the noise out of the way, so that subsequent commits can have a higher proportion of signal.
-
- Oct 21, 2018
-
-
Simon Tatham authored
Previously, it returned a human-readable string suitable for log files, which tried to say something useful about the remote end of a socket. Now it returns a whole SocketPeerInfo structure, of which that human-friendly log string is just one field, but also some of the same information - remote IP address and port, in particular - is provided in machine-readable form where it's available.
-
Simon Tatham authored
That's more directly useful in uxpty.c (which is currently the only actual client of the function), and also matches the data that SSH clients send in "pty-req". Also, it makes that method behave more like the GUI query function get_window_pixels used by terminal.c (with the sole exception that unlike g_w_p it's allowed to return failure), so it becomes even more trivial to implement in the GUI front ends.
-
Simon Tatham authored
A function to compare two strings _both_ in ptrlen form (I've had ptrlen_eq_string for ages, but for some reason, never quite needed ptrlen_eq_ptrlen). A function to ask whether one ptrlen starts with another (and, optionally, return a ptrlen giving the remaining part of the longer string). And the va_list version of logeventf, which I really ought to have written in the first place by sheer habit, even if it was only needed by logeventf itself.
-
- Oct 15, 2018
-
-
Simon Tatham authored
One to make one from a NUL-terminated string, and another to make one from a strbuf. I've switched over all the obvious cases where I should have been using these functions.
-
- Oct 11, 2018
-
-
Simon Tatham authored
This is a new vtable-based abstraction which is passed to a backend in place of Frontend, and it implements only the subset of the Frontend functions needed by a backend. (Many other Frontend functions still exist, notably the wide range of things called by terminal.c providing platform-independent operations on the GUI terminal window.) The purpose of making it a vtable is that this opens up the possibility of creating a backend as an internal implementation detail of some other activity, by providing just that one backend with a custom Seat that implements the methods differently. For example, this refactoring should make it feasible to directly implement an SSH proxy type, aka the 'jump host' feature supported by OpenSSH, aka 'open a secondary SSH session in MAINCHAN_DIRECT_TCP mode, and then expose the main channel of that as the Socket for the primary connection'. (Which of course you can already do by spawning 'plink -nc' as a separate proxy process, but this would permit it in the _same_ process without anything getting confused.) I've centralised a full set of stub methods in misc.c for the new abstraction, which allows me to get rid of several annoying stubs in the previous code. Also, while I'm here, I've moved a lot of duplicated modalfatalbox() type functions from application main program files into wincons.c / uxcons.c, which I think saves duplication overall. (A minor visible effect is that the prefixes on those console-based fatal error messages will now be more consistent between applications.)
-
- Oct 10, 2018
-
-
Simon Tatham authored
LogContext is now the owner of the logevent() function that back ends and so forth are constantly calling. Previously, logevent was owned by the Frontend, which would store the message into its list for the GUI Event Log dialog (or print it to standard error, or whatever) and then pass it _back_ to LogContext to write to the currently open log file. Now it's the other way round: LogContext gets the message from the back end first, writes it to its log file if it feels so inclined, and communicates it back to the front end. This means that lots of parts of the back end system no longer need to have a pointer to a full-on Frontend; the only thing they needed it for was logging, so now they just have a LogContext (which many of them had to have anyway, e.g. for logging SSH packets or session traffic). LogContext itself also doesn't get a full Frontend pointer any more: it now talks back to the front end via a little vtable of its own called LogPolicy, which contains the method that passes Event Log entries through, the old askappend() function that decides whether to truncate a pre-existing log file, and an emergency function for printing an especially prominent message if the log file can't be created. One minor nice effect of this is that console and GUI apps can implement that last function subtly differently, so that Unix console apps can write it with a plain \n instead of the \r\n (harmless but inelegant) that the old centralised implementation generated. One other consequence of this is that the LogContext has to be provided to backend_init() so that it's available to backends from the instant of creation, rather than being provided via a separate API call a couple of function calls later, because backends have typically started doing things that need logging (like making network connections) before the call to backend_provide_logctx. Fortunately, there's no case in the whole code base where we don't already have logctx by the time we make a backend (so I don't actually remember why I ever delayed providing one). So that shortens the backend API by one function, which is always nice. While I'm tidying up, I've also moved the printf-style logeventf() and the handy logevent_and_free() into logging.c, instead of having copies of them scattered around other places. This has also let me remove some stub functions from a couple of outlying applications like Pageant. Finally, I've removed the pointless "_tag" at the end of LogContext's official struct name.
-
- Oct 06, 2018
-
-
Simon Tatham authored
Ian Jackson points out that the Linux kernel has a macro of this name with the same purpose, and suggests that it's a good idea to use the same name as they do, so that at least some people reading one code base might recognise it from the other. I never really thought very hard about what order FROMFIELD's parameters should go in, and therefore I'm pleasantly surprised to find that my order agrees with the kernel's, so I don't have to permute every call site as part of making this change :-)
-
- Sep 24, 2018
-
-
Simon Tatham authored
The callback has the same semantics as for packet queues: it triggers automatically when data is added to a bufchain, not when it's removed.
-
- Sep 20, 2018
-
-
Simon Tatham authored
Now there's a centralised routine in misc.c to do the sanitisation, which copies data on to an outgoing bufchain. This allows me to remove from_backend_untrusted() completely from the frontend API, simplifying code in several places. Two use cases for untrusted-terminal-data sanitisation were in the terminal.c prompts handler, and in the collection of SSH-2 userauth banners. Both of those were writing output to a bufchain anyway, so it was very convenient to just replace a bufchain_add with sanitise_term_data and then not have to worry about it again. There was also a simplistic sanitiser in uxcons.c, which I've now replaced with a call to the good one - and in wincons.c there was a FIXME saying I ought to get round to that, which now I have!
-
Simon Tatham authored
This should make it easier to do formatted-string based logging outside ssh.c, because I can wrap up a local macro in any source file I like that expands to logevent_and_free(wherever my Frontend is, dupprintf(macro argument)). It caused yet another stub function to be needed in testbn, but there we go. (Also, while I'm here, removed a redundant declaration of logevent itself from ssh.h. The one in putty.h is all we need.)
-