I've done this on a 'where possible' basis: in Windows paletted mode
(in case anyone is still using an old enough graphics card to need
that!) I simply haven't bothered, and will completely ignore the dim
flag.
Markus Gans points out that some applications which (not at all
unreasonably) don't trust $TERM to tell them the full capabilities of
their terminal will use the sequence "OSC 4 ; nn ; ? BEL" to ask for
the colour-palette value in position nn, and they may not particularly
care _what_ the results are but they will use them to decide whether
the right number of colour palette entries even exist.
Otherwise, moving the cursor (at least in active, filled-cell mode) on
to a true-coloured character cell causes it to vanish completely
because the cell's colours override the thing that differentiates the
cursor.
I'm not sure if any X11 monochrome visuals or Windows paletted display
modes are still around, but just in case they are, we shouldn't
attempt true colour on either kind of display.
This is a heavily rewritten version of a patch originally by Lorenz
Diener; it was tidied up somewhat by Christian Brabandt, and then
tidied up more by me. The basic idea is to add to the termchar
structure a pair of small structs encoding 24-bit RGB values, each
with a flag indicating whether it's turned on; if it is, it overrides
any other specification of fg or bg colour for that character cell.
I've added a test line to colours.txt containing a few example colours
from /usr/share/X11/rgb.txt. In fact it makes quite a good demo to run
the whole of rgb.txt through this treatment, with a command such as
perl -pe 's!^\s*(\d+)\s+(\d+)\s+(\d+).*$!\e[38;2;$1;$2;$3m$&\e[m!' rgb.txt
Ilya Shipitsin sent me a list of errors reported by a tool 'cppcheck',
which I hadn't seen before, together with some fixes for things
already taken off that list. This change picks out all the things from
the remaining list that I could quickly identify as actual errors,
which it turns out are all format-string goofs along the lines of
using a %d with an unsigned int, or a %u with a signed int, or (in the
cases in charset/utf8.c) an actual _size_ mismatch which could in
principle have caused trouble on a big-endian target.
This too is not in the list of known DLLs on Windows 10. I don't know
of any actual viable hijacking attack based on it, which according to
my reading of MSDN (specifically, a rather vague hint in
https://msdn.microsoft.com/library/ff919712) _may_ be because we
mention the common controls assembly in our application manifest; but
better safe than sorry.
Now the entire list of remaining DLLs that PuTTY links against at load
time is a subset of the Win10 known DLLs list, so that _should_ mean
that everything we load before we've deployed our own defence
(SetDefaultDllDirectories) is defended against for us by Windows
itself.
It's not on the default list of important system 'known DLLs' stored
at HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\KnownDLLs (see
https://isc.sans.edu/forums/diary/DLL+hijacking+vulnerabilities/9445/ )
which apparently makes it exempt from Windows's standard DLL hijacking
defence, i.e. if an executable links against it in the normal way then
that executable will be vulnerable to DLL hijacking from a file called
winmm.dll in the same directory as it.
The solution is to load it dynamically _after_ we've locked down our
DLL search path, which fortunately PuTTY's code base is well used to
doing already for other DLLs.
I think all of the cases in this switch must have originally said
(shift_state ? 'this' : 'that'), and in all but the VK_NUMPAD5 case
the two options were different, and I left VK_NUMPAD5 containing a
redundant ?: just to make it line up in a nice table with the others.
But now the others all have more options than that because I had to
support Ctrl as well as Shift modifiers, so there's no reason to have
that silly ?: lingering around (and it annoys Coverity).
Avoided referring to some functions and header files that aren't there
in the winelib world (_vsnprintf, _stricmp, SecureZeroMemory,
multimon.h), and worked around a really amazingly annoying issue in
which Winelib objects to you using the type 'fd_set' unless you
included winsock2.h before stdlib.h.
Partly to reassure the user that they got what they asked for, and
partly so that's a clue for us in the logs when we get bug reports.
This involved repurposing platform_psftp_post_option_setup() (no longer
used since e22120fe) as platform_psftp_pre_conn_setup(), and moving it
to after logging is set up.
These are benign, I think. clang warns about casting non-pointer-sized
integers to pointers, but the Windows API actually does sometimes
involve values that are either pointers or _small_ integers, so in the
two cases involved I just cast through ULONG_PTR to silence the
warning. And clang insists that the integer whose address I give to
sk_getxdmdata is still uninitialised afterwards, which is just a lie.
This change applies to every situation when GUI PuTTY knowingly spawns
another GUI PuTTY, to wit, the System menu options 'New Session',
'Duplicate Session' and the 'Saved Sessions' submenu.
(Literally speaking, what we actually pass through to the sub-PuTTY's
command line is not the "-restrict-acl" option itself, but a special
prefix "&R", which has the same meaning but which lives in the special
pre-argv-splitting command-line namespace like the magic options used
for Duplicate Session and the old '@sessionname' prefix which the
Saved Sessions submenu still uses. Otherwise, by the time we split up
argv and recognised -restrict-acl, it would be too late to parse those
other options.)
One case in which PuTTY spawns a subprocess and this change _doesn't_
apply is when the subprocess is a proxy command which happens to be a
Plink. Recognising Plink commands in that situation would be fragile
and unreliable, and in any case if the user wants a proxy Plink to be
ACL-restricted, they are in control of its exact command line so they
can add -restrict-acl themselves.
These ones are stylistic rather than potential bugs: mostly signedness
of char pointers in cases where they clearly aren't going to cause the
wrong thing to actually happen, and one thing in winsecur.c where
clang would have preferred an extra pair of braces around some
initialisers but it's legal with or without. But since some of clang's
warnings turn out to be quite useful, it seems worth silencing these
harmless ones so as to be able to see the rest.
As documented in bug 'win-process-acl-finesse', we've had enough
assorted complaints about it breaking various non-malicious pieces of
Windows process interaction (ranging from git->plink integration to
screen readers for the vision-impaired) that I think it's more
sensible to set the process back to its default level of protection.
This precaution was never a fully effective protection anyway, due to
the race condition at process startup; the only properly effective
defence would have been to prevent malware running under the same user
ID as PuTTY in the first place, so in that sense, nothing has changed.
But people who want the arguable defence-in-depth advantage of the ACL
restriction can now turn it on with the '-restrict-acl' command-line
option, and it's up to them whether they can live with the assorted
inconveniences that come with it.
In the course of this change, I've centralised a bit more of the
restriction code into winsecur.c, to avoid repeating the error
handling in multiple places.
The algorithm Windows uses to generate AppUserModelIDs "hangs on" to
removable media (CDs/DVDs) if PuTTY is launched with a CD/DVD in a drive.
Set the AppUserModelID explicitly to avoid using this algorithm.
At least on systems providing SetDefaultDllDirectories, this should
stop PuTTY from being willing to load DLLs from its containing
directory - which makes no difference when it's been properly
installed (in which case the application dir contains no DLLs anyway),
but does if it's being run from somewhere uncontrolled like a browser
downloads directory.
Preliminary testing suggests that this shouldn't break any existing
deliberate use of DLLs, including GSSAPI providers.
Previously only Unix front ends bothered to include it, on the basis
that only the pty backend needed it (to set IUTF8 in the pty). We're
about to need it everywhere else too.
These integer types are correct for the id/handle parameter to
AppendMenu / InsertMenu / DeleteMenu, and also for the return type of
dialog box procedures.
We also have the special-purpose -DUNPROTECT to disable just the ACL
changes, but if you want to compile without any Windows security API
support at all (e.g. experimentally building against winelib) then
it's easier not to have to specify both defines separately.
logevent() doesn't do printf-style formatting (though the logeventf
wrapper in ssh.c does), so if you need to format a message, it has to
be done separately with dupprintf.
By default Windows processes have wide open ACLs which allow interference
by other processes running as the same user. Adjust our ACL to make this
a bit harder.
Because it's useful to protect PuTTYtel as well, carve winsecur.c into
advapi functions and wincapi.c for crypt32 functions.
This is generated in response to the SendInput() Windows API call, if
that in turn is passed an KEYBDINPUT structure with KEYEVENTF_UNICODE
set. That method of input generation is used by programs such as
'WinCompose' to send an arbitrary Unicode character as if it had been
typed at the keyboard, even if the keyboard doesn't actually provide a
key for it.
Like VK_PROCESSKEY, this key code is an exception to our usual policy
of manually translating keystrokes: we handle it by calling
TranslateMessage, to get back the Unicode character it contains as a
WM_CHAR message.
(If that Unicode character in turn is outside the BMP, it may come
back as a pair of WM_CHARs in succession containing UTF-16 surrogates;
if so, that's OK, because the new Unicode WM_CHAR handler can cope.)
This causes WM_CHAR messages sent to us to have a wParam containing a
16-bit value encoded in UTF-16, rather than an 8-bit value encoded in
the system code page.
As far as I can tell, there aren't many other knock-on effects - e.g.
you can still interact with the window using ordinary char-based API
functions such as SetWindowText, and the Windows API will do the
necessary conversions behind the scenes. However, even so, I'm half
expecting some sort of unforeseen bug to show up as a result of this.
Having found a lot of unfixed constness issues in recent development,
I thought perhaps it was time to get proactive, so I compiled the
whole codebase with -Wwrite-strings. That turned up a huge load of
const problems, which I've fixed in this commit: the Unix build now
goes cleanly through with -Wwrite-strings, and the Windows build is as
close as I could get it (there are some lingering issues due to
occasional Windows API functions like AcquireCredentialsHandle not
having the right constness).
Notable fallout beyond the purely mechanical changing of types:
- the stuff saved by cmdline_save_param() is now explicitly
dupstr()ed, and freed in cmdline_run_saved.
- I couldn't make both string arguments to cmdline_process_param()
const, because it intentionally writes to one of them in the case
where it's the argument to -pw (in the vain hope of being at least
slightly friendly to 'ps'), so elsewhere I had to temporarily
dupstr() something for the sake of passing it to that function
- I had to invent a silly parallel version of const_cmp() so I could
pass const string literals in to lookup functions.
- stripslashes() in pscp.c and psftp.c has the annoying strchr nature
This was an old bug, fixed around 0.59, which apparently regressed
when I rewrote the main event loop using the toplevel_callback
mechanism.
Investigation just now suggests that it has to do with my faulty
assumption that Windows PeekMessage would deliver messages in its
message queue in FIFO order (i.e. that the thing calling itself a
message queue is actually a _queue_). In fact my WM_NETEVENT seems to
like to jump the queue, so that once a steady stream of them starts
arriving, we never do anything else in the main event loop (except
deal with handles).
Worked around in a simple and slightly bodgy way, namely, we don't
stop looping on PeekMessage and run our toplevel callbacks until we've
either run out of messages completely or else seen at least one that
_isn't_ a WM_NETEVENT. That way we should reliably interleave NETEVENT
processing with processing of other stuff.
I'm not actually sure why we've always had back ends notify ldisc of
changes to echo/edit settings by giving ldisc_send(ldisc,NULL,0,0) a
special meaning, instead of by having a separate dedicated notify
function with its own prototype and parameter set. Coverity's recent
observation that the two kinds of call don't even have the same
requirements on the ldisc (particularly, whether ldisc->term can be
NULL) makes me realise that it's really high time I separated the two
conceptually different operations into actually different functions.
While I'm here, I've renamed the confusing ldisc_update() function
which that special operation ends up feeding to, because it's not
actually a function applying to an ldisc - it applies to a front end.
So ldisc_send(ldisc,NULL,0,0) is now ldisc_echoedit_update(ldisc), and
that in turn figures out the current echo/edit settings before passing
them on to frontend_echoedit_update(). I think that should be clearer.
The IDM_RECONF handler unconditionally calls ldisc_configure to
reconfigure the line discipline for the new echo/edit settings, but in
fact ldisc can be NULL if no session is currently active. (Indeed, the
very next line acknowledges this, by testing it for NULL before
calling ldisc_send!) Thanks to Alexander Wong for the report.
[originally from svn r10214]
On Windows (X mouse reporting of the mouse wheel isn't currently done
by the Unix front end, though I'm shortly about to fix that too) a
mouse wheel event is translated into a virtual button, and we send
both a press and a release of that button to terminal.c, which encodes
both in X mouse reporting escape sequences and passes them on to the
server. This isn't consistent with what xterm does - scroll-wheel
events are encoded _like_ button presses, but differ semantically in
that they don't have matching releases. So we're updating to match
xterm.
[originally from svn r10138]
I've gone through everywhere we handle host names / addresses (on
command lines, in PuTTY config, in port forwarding, in X display
names, in host key storage...) and tried to make them handle IPv6
literals sensibly, by using the host_str* functions I introduced in my
previous commit. Generally it's now OK to use a bracketed IPv6 literal
anywhere a hostname might have been valid; in a few cases where no
ambiguity exists (e.g. no :port suffix is permitted anyway)
unbracketed IPv6 literals are also acceptable.
[originally from svn r10120]
The basic strategy is described at the top of the new source file
sshshare.c. In very brief: an 'upstream' PuTTY opens a Unix-domain
socket or Windows named pipe, and listens for connections from other
PuTTYs wanting to run sessions on the same server. The protocol spoken
down that socket/pipe is essentially the bare ssh-connection protocol,
using a trivial binary packet protocol with no encryption, and the
upstream has to do some fiddly transformations that I've been
referring to as 'channel-number NAT' to avoid resource clashes between
the sessions it's managing.
This is quite different from OpenSSH's approach of using the Unix-
domain socket as a means of passing file descriptors around; the main
reason for that is that fd-passing is Unix-specific but this system
has to work on Windows too. However, there are additional advantages,
such as making it easy for each downstream PuTTY to run its own
independent set of port and X11 forwardings (though the method for
making the latter work is quite painful).
Sharing is off by default, but configuration is intended to be very
easy in the normal case - just tick one box in the SSH config panel
and everything else happens automatically.
[originally from svn r10083]
This restores PuTTY's backward compatibility to versions of Windows
too old to have ToUnicodeEx in their system libraries, which was
accidentally broken in 0.63.
[originally from svn r10061]
A couple of users report that my recent reworking of the Windows
top-level message loop has led to messages occasionally being lost,
and MsgWaitForMultipleObjects blocking when it ought to have been
called with a zero timeout. I haven't been able to reproduce this
myself, but according to one reporter, PeekMessage(PM_NOREMOVE) is
effective at checking for a non-empty message queue in a way that
GetQueueStatus is not. Switch to using that instead. Thanks to Eric
Flumerfelt for debugging and testing help.
[originally from svn r10057]
Jochen Erwied points out that once you've used PeekMessage to remove
_one_ message from the message queue, MsgWaitForMultipleObjects will
consider the whole queue to have been 'read', or at least looked at
and deemed uninteresting, and so it will block until a further message
comes in. Hence, my change in r10040 which stops us from looping on
PeekMessage until the queue is empty has the effect of causing the
rest of the message queue not to be acted on until a new message comes
in to unblock it. Fix by checking if the queue is nonempty in advance
of calling MsgWaitForMultipleObjects, and if so, giving it a zero
timeout just as we do if there's a pending toplevel callback.
[originally from svn r10052]
[r10040 == 5c4ce2fadf]
This change attempts to reinstate as a universal property something
which was sporadically true of the ad-hockery that came before
toplevel callbacks: that if there's a _very long_ queue of things to
be done through the callback mechanism, the doing of them will be
interleaved with re-checks of other event sources, which might (e.g.)
cause a flag to be set which makes the next callback decide not to do
anything after all.
[originally from svn r10040]
It was one of those things that went in ages ago on Windows and never
got replicated in the Unix front end. And it needn't be: ldisc.c is a
perfect place to put it, since it knows which of the data it's sending
is based on a keystroke and which is automatically generated, and it
also has access to the terminal context. So now a keypress can
interrupt a runaway paste on all platforms.
[originally from svn r10025]
Again, I've removed the special-purpose ad-hockery from the assorted
front end message loops that dealt with deferred handling of socket
errors, and instead uxnet.c and winnet.c arrange that for themselves
by calling the new general top-level callback mechanism.
[originally from svn r10023]
Instead of setting a must_close_session flag and having special code
in the message loop to check it, we'll schedule the call to
close_session using the new top-level callback system.
[originally from svn r10021]
I've removed the ad-hoc front-end bodgery in the Windows and GTK ports
to arrange for term_paste to be called at the right moments, and
instead, terminal.c itself deals with knowing when to send the next
chunk of pasted data using a combination of timers and the new
top-level callback mechanism.
As a happy side effect, it's now all in one place so I can actually
understand what it's doing! It turns out that what all that confusing
code was up to is: send a line of pasted data, and delay sending the
next line until either a CR or LF is returned from the server
(typically indicating that the pasted text has been received and
echoed) or 450ms elapse, whichever comes first.
[originally from svn r10020]
This is a little like schedule_timer, in that the callback you provide
will be run from the top-level message loop of whatever application
you're in; but unlike the timer mechanism, it will happen
_immediately_.
The aim is to provide a general way to avoid re-entrance of code, in
cases where just _doing_ the thing you want done is liable to trigger
a confusing recursive call to the function in which you came to the
decision to do it; instead, you just request a top-level callback at
the message loop's earliest convenience, and do it then.
[originally from svn r10019]
PuTTY does not trim a colon suffix off the hostname if it contains
_more than one_ colon. This allows IPv6 literals to be entered.
(Really we need to do a much bigger revamp of all uses of hostnames to
arrange that square-bracketed IPv6 literals work consistently, but
this at least removes a regression over 0.62.)
[originally from svn r9983]
[r9214 == a1f3b7a358]
palette_set() to be bogus. Fortunately, this isn't exploitable through
the terminal emulator, because the palette escape sequence parser
contains its own bounds check before even calling palette_set().
While I'm at it, fix the same goof in the OS X version! That port is
more or less abandoned, but that's no excuse for leaving obviously
wrong code lying around.
[originally from svn r9965]
The most interesting one is printer_add_enum, which I've modified to
take a char ** rather than a char * so that it can both realloc its
input buffer _and_ return NULL to indicate error.
[originally from svn r9959]
