Simon Tatham [Mon, 26 Aug 2013 11:55:56 +0000 (11:55 +0000)]
Fix free of an uninitialised pointer.
CHAN_AGENT channels need c->u.a.message to be either NULL or valid
dynamically allocated memory, because it'll be freed by
ssh_channel_destroy. This bug triggers if an agent forwarding channel
is opened and closed without having sent any queries.
Simon Tatham [Thu, 22 Aug 2013 17:45:26 +0000 (17:45 +0000)]
Fix handling of IPv6 dynamic forwardings.
During the Conf revamp, I changed the internal representation of
dynamic forwardings so that they were stored as the conceptually
sensible L12345=D rather than the old D12345, and added compensation
code to translate to the latter form for backwards-compatible data
storage and for OpenSSH-harmonised GUI display. Unfortunately I forgot
that keys in the forwarding data can also prefix the L/R with a
character indicating IPv4/IPv6, and my translations didn't take
account of that possibility. Fix them.
Simon Tatham [Sun, 18 Aug 2013 06:48:20 +0000 (06:48 +0000)]
Sensibly enforce non-interactive rekeying.
We now only present the full set of host key algorithms we can handle
in the first key exchange. In subsequent rekeys, we present only the
host key algorithm that we agreed on the previous time, and then we
verify the host key by simply enforcing that it's exactly the same as
the one we saw at first and disconnecting rudely if it isn't.
Simon Tatham [Sat, 17 Aug 2013 16:06:40 +0000 (16:06 +0000)]
Make calling term_nopaste() a cross-platform feature.
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.
Simon Tatham [Sat, 17 Aug 2013 16:06:27 +0000 (16:06 +0000)]
Revamp net_pending_errors using toplevel callbacks.
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.
Simon Tatham [Sat, 17 Aug 2013 16:06:22 +0000 (16:06 +0000)]
Revamp GTK's session close handling using toplevel callbacks.
Instead of having a special GTK idle function for dealing with session
closing, I now use the new top-level callback mechanism which is
slightly simpler for calling a one-off function.
Also in this commit, I've arranged for connection_fatal to queue a
call to the same session close function after displaying the message
box, with the effect that now all the same processing takes place no
matter whether the session closes cleanly or uncleanly - e.g. the SSH
specials submenu is cleaned out, as it should be.
Simon Tatham [Sat, 17 Aug 2013 16:06:18 +0000 (16:06 +0000)]
Revamp Windows's close_session() using toplevel callbacks.
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.
Simon Tatham [Sat, 17 Aug 2013 16:06:12 +0000 (16:06 +0000)]
Revamp the terminal paste mechanism using toplevel callbacks.
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.
Simon Tatham [Sat, 17 Aug 2013 16:06:08 +0000 (16:06 +0000)]
Add a general way to request an immediate top-level callback.
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.
Simon Tatham [Thu, 15 Aug 2013 06:42:36 +0000 (06:42 +0000)]
Sebastian Kuschel reports that pfd_closing can be called for a socket
error with pr->c NULL, in which case calling sshfwd_unclean_close on
it will dereference NULL and segfault. Write an alternative error
handling path for that possibility.
(I don't know if it's the only way, but one way this can happen is if
you're doing dynamic forwarding and the socket error occurs during
SOCKS negotiation, in which case no SSH channel has been set up yet
because we haven't yet found out what we want to put in the
direct-tcpip channel open message.)
Simon Tatham [Tue, 13 Aug 2013 06:46:51 +0000 (06:46 +0000)]
It turns out I was a little over-strict in my handling of EOF in
pscp.c when I did the big revamp in r9279: I assumed that in any SCP
connection we would be the first to send EOF, but in fact this isn't
true - doing downloads with old-SCP, EOF is initiated by the server,
so we were spuriously reporting an error for 'unexpected' EOF when
everything had gone fine. Thanks to Nathan Phelan for the report.
Simon Tatham [Thu, 8 Aug 2013 17:22:07 +0000 (17:22 +0000)]
sbcsgen.pl uses 'select' to point Perl at a different default output
handle. Revert that when we hackily call it from mkfiles.pl, so that
if I have a need to insert diagnostics in the latter they won't go
into the end of sbcsdat.c.
Simon Tatham [Wed, 7 Aug 2013 06:22:52 +0000 (06:22 +0000)]
Revert the default for font bolding style back to using colours rather
than fonts. I broke this in r9559 when I added the option for 'both',
because the internal representation got offset by one so as to change
from a boolean to two bitfields and I must have confused myself about
what the default should be.
Simon Tatham [Mon, 5 Aug 2013 19:50:51 +0000 (19:50 +0000)]
The bignum code has two representations of zero, since
bn_restore_invariant (and the many loops that duplicate it) leaves a
single zero word in a bignum representing 0, whereas the constant
'Zero' does not have any data words at all. Cope with this in
bignum_cmp.
(It would be a better plan to decide on one representation and stick
with it, but this is the less disruptive fix for the moment.)
Simon Tatham [Sun, 4 Aug 2013 19:34:10 +0000 (19:34 +0000)]
Spot when we didn't successfully create an RSA public key from a
public blob, and return a proper error in that situation rather than a
struct with unhelpful NULLs in.
Simon Tatham [Sun, 4 Aug 2013 19:33:53 +0000 (19:33 +0000)]
More consistently defend against division by zero with assertions. We
now check that all the modular functions (modpow, modinv, modmul,
bigdivmod) have nonzero moduli, and that modinv also has a nonzero
thing to try to invert.
Simon Tatham [Sun, 4 Aug 2013 19:32:10 +0000 (19:32 +0000)]
Reinstate a piece of code accidentally removed in r9214, where Windows
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.)
Simon Tatham [Fri, 2 Aug 2013 22:33:40 +0000 (22:33 +0000)]
Raise the default scrollback from 200 to 2000 lines. The former was
not so silly in the 1990s and before I implemented scrollback
compression, but it's been a ridiculously low default for a while now.
Simon Tatham [Mon, 29 Jul 2013 17:47:33 +0000 (17:47 +0000)]
Remove one of the frees added in r9916. stat_name points to somewhere
within the same string that destfname points to the start of, so
freeing it causes at best a double-free of destfname and more likely a
free of something that isn't even the start of an allocated block.
Simon Tatham [Wed, 24 Jul 2013 19:18:06 +0000 (19:18 +0000)]
Get rid of the variable 'advapi' in Pageant's WinMain, which was never
actually used for anything sensible and could have been freed while
containing nonsense at program end.
Simon Tatham [Mon, 22 Jul 2013 07:12:31 +0000 (07:12 +0000)]
Increase FONT_MAXNO from 0x2f to 0x40, to ensure the fonts[] array
includes every possible combination of the font bitfields, in
particular ATTR_OEM|ATTR_NARROW.
Simon Tatham [Mon, 22 Jul 2013 07:12:26 +0000 (07:12 +0000)]
Correct an inequality sign causing the bounds check in Windows
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.
Simon Tatham [Mon, 22 Jul 2013 07:12:05 +0000 (07:12 +0000)]
Rationalise null pointer checks in both decode_codepage functions, so
that decode_codepage(NULL) and decode_codepage("") both return the
default character set.
Simon Tatham [Mon, 22 Jul 2013 07:11:58 +0000 (07:11 +0000)]
Fix a double error handling goof in the winstore side of the jump list
support: transform_jumplist_registry should give its caller
dynamically allocated data if and only if it returns JUMPLISTREG_OK,
and get_jumplist_registry_entries should test the return value against
JUMPLISTREG_OK rather than a value from a totally different enum.
Simon Tatham [Mon, 22 Jul 2013 07:11:54 +0000 (07:11 +0000)]
Another big batch of memory leak fixes, again mostly on error paths.
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.
Simon Tatham [Mon, 22 Jul 2013 07:11:44 +0000 (07:11 +0000)]
Report an error if deleting a random seed file fails.
(This has also required me to add a currently unused nonfatal() to
PuTTYgen, since although PuTTYgen won't actually try to delete
putty.rnd, it does link in winstore.c as a whole.)
Simon Tatham [Mon, 22 Jul 2013 07:11:39 +0000 (07:11 +0000)]
Invent a win_strerror() function which behaves as much like Unix
strerror as I can arrange, wrapping up all the ugly FormatMessage
nonsense and caching previously looked-up messages for reuse so that
callers can treat them as static.
Simon Tatham [Sun, 21 Jul 2013 10:12:58 +0000 (10:12 +0000)]
If the SSH server sends us CHANNEL_CLOSE for a channel on which we're
sitting on a pile of buffered data waiting for WINDOW_ADJUSTs, we
should throw away that buffered data, because the CHANNEL_CLOSE tells
us that we won't be receiving those WINDOW_ADJUSTs, and if we hang on
to the data and keep trying then it'll prevent ssh_channel_try_eof
from sending the CHANNEL_EOF which is a prerequisite of sending our
own CHANNEL_CLOSE.
Simon Tatham [Sun, 21 Jul 2013 09:16:37 +0000 (09:16 +0000)]
Add '.so' to the list of file extensions cleared up by 'make clean' in
Makefile.cyg, since if you're building against Winelib it will
generate one of those alongside each .exe file.
Simon Tatham [Sun, 21 Jul 2013 07:40:36 +0000 (07:40 +0000)]
Completely remove the 'frozen_readable' mechanism from uxnet.c. It
parallels a similar mechanism in winnet.c and came over by copy and
paste, but is pointless in the Unix networking API.
On Windows, if you're using a mechanism such as WSAAsyncSelect which
delivers readability notifications as messages rather than return
values from a system call, you only get notified that a socket is
readable once - it remembers that it's told you, and doesn't tell you
again until after you've done a read. So in the case where we
intentionally stop reading from a socket because our local buffer is
full, and later want to start reading again, we do a read from the
socket with MSG_PEEK set, and that clears Windows's flag and tells it
to start sending us readability notifications again.
On Unix, select() and friends didn't do anything so strange in the
first place, so the whole mechanism is unnecessary.
Simon Tatham [Sat, 20 Jul 2013 13:15:11 +0000 (13:15 +0000)]
Another two mis-fixes from r9919: when we sfree(line) on exit from the
ssh.com and OpenSSH key import loops, we should also null it out so
that the cleanup path doesn't try to re-free the same pointer.
Simon Tatham [Sat, 20 Jul 2013 13:15:10 +0000 (13:15 +0000)]
Redo a mis-fix of a memory leak in r9919: I added sfree(data)
immediately after conf_deserialise in the Duplicate Session receiver,
whereas I should have put it after the subsequent loop that extracts
the pty argv if any.
Simon Tatham [Sat, 20 Jul 2013 11:31:24 +0000 (11:31 +0000)]
Switch to translating keystrokes using ToUnicodeEx rather than
ToAsciiEx, where possible.
This enables support for keys which generate Unicode characters that
aren't in the system code page, which seems to me like a perverse way
for Windows to have set up the system code page but apparently does
happen, e.g. (I'm told) U+0219 and U+021B on Romanian keyboards.
Simon Tatham [Sat, 20 Jul 2013 08:34:54 +0000 (08:34 +0000)]
Been meaning to get round to this for a while: use CryptGenRandom to
gather extra entropy at Windows PuTTY startup time. (It's only used as
one of the inputs to PuTTY's internal entropy pool, so nobody is
required to trust it.)
Simon Tatham [Fri, 19 Jul 2013 17:44:58 +0000 (17:44 +0000)]
Add some conditionally-compilable diagnostics to the RNG. I got
briefly worried that it might not be doing what I thought it was
doing, but examining these diagnostics shows that it is after all, and
now I've written them it would be a shame not to keep them for future
use.
Simon Tatham [Fri, 19 Jul 2013 17:44:28 +0000 (17:44 +0000)]
Add a nonfatal() function everywhere, to be used for reporting things
that the user really ought to know but that are not actually fatal to
continued operation of PuTTY or a single network connection.
Simon Tatham [Sun, 14 Jul 2013 17:08:35 +0000 (17:08 +0000)]
In the various channel request mini-coroutines, replace
crWaitUntilV(pktin) with plain crReturnV, because those coroutines can
be called back either with a response packet from the channel request
_or_ with NULL by ssh_free meaning 'please just clean yourself up'.
Simon Tatham [Sun, 14 Jul 2013 10:46:55 +0000 (10:46 +0000)]
Remove a redundant while-loop condition when reading RFC822-style
header text from a PuTTY key file.
(It's silly to have both while (len > 0) at the top of the loop _and_
an if (len == 0) return in the middle, and in fact the former was the
erroneous one since it would have prohibited a 39-character header,
which I intended to be permitted.)
Simon Tatham [Sun, 14 Jul 2013 10:46:39 +0000 (10:46 +0000)]
Remove a return path from sshcom_write() which was both unreachable
(it would trigger if !type==RSA and !type==DSA, but one of those must
have been true to get there in the first place) and erroneous (it
would return NULL without going through the cleanup code). Since the
code's internal structure guarantees that path isn't reached, replace
it with an assert.
Simon Tatham [Sun, 14 Jul 2013 10:46:34 +0000 (10:46 +0000)]
Use the new ctrl_alloc_with_free to clean up a long-standing FIXME in
the session saving code, in which the contents of the edit box giving
the current saved session name was stored in a horrid place with a
fixed length. Now it's dangling off sessionsaver_data as it always
ought to have been, and it's dynamically reallocated to the
appropriate length, and there's a free function that cleans it up at
the end of the dialog's lifetime.
Simon Tatham [Sun, 14 Jul 2013 10:46:29 +0000 (10:46 +0000)]
Add an extended version of ctrl_alloc which permits you to provide a
custom free function, in case you need to ctrl_alloc a structure which
then has additional dynamically allocated things dangling off it.
Simon Tatham [Sun, 14 Jul 2013 10:46:27 +0000 (10:46 +0000)]
Move the calculation of the exchange hash to above the various
warnings about insecure crypto components. The latter may crReturn
(though not in any current implementation, I believe), which
invalidates pktin, which is used by the former.
Simon Tatham [Sun, 14 Jul 2013 10:46:17 +0000 (10:46 +0000)]
Add a missing null pointer check in wc_unescape, to bring it in line
with the usage comment saying you're allowed to pass NULL to find out
only the return value. No caller actually does pass NULL at the
moment.
Simon Tatham [Sun, 14 Jul 2013 10:45:54 +0000 (10:45 +0000)]
Tighten up a lot of casts from unsigned to int which are read by one
of the GET_32BIT macros and then used as length fields. Missing bounds
checks against zero have been added, and also I've introduced a helper
function toint() which casts from unsigned to int in such a way as to
avoid C undefined behaviour, since I'm not sure I trust compilers any
more to do the obviously sensible thing.
Simon Tatham [Thu, 11 Jul 2013 17:43:41 +0000 (17:43 +0000)]
Add an assortment of missing frees, and one missing file close. Mostly
on error paths, although the one in PSFTP's wildcard_iterate will come
up in normal usage.
Simon Tatham [Thu, 11 Jul 2013 17:24:53 +0000 (17:24 +0000)]
xfer_{up,down}load_gotpkt free their input sftp_packet as a side
effect of handling it, but they do not free it if it isn't a packet
they recognise as part of their upload/download. Invent a return value
that specifically signals this, and consistently free pktin at every
call site if that return value comes back. Also, ensure that that
return value also always comes with something meaningful in fxp_error.
Simon Tatham [Thu, 11 Jul 2013 17:24:47 +0000 (17:24 +0000)]
Fix a collection of calls to tell_user so that they don't add their
own newline before the one tell_user puts on the end anyway. Also,
while I'm here, make up my mind about whether to prefix messages with
"scp:" or "pscp:" - I choose the latter.
Simon Tatham [Thu, 11 Jul 2013 17:24:44 +0000 (17:24 +0000)]
Fix a couple of code paths on which, if fxp_readdir returned an error,
we would return without first closing the directory handle we had used
as an argument.
Simon Tatham [Thu, 11 Jul 2013 17:24:39 +0000 (17:24 +0000)]
It's not actually legal by the C standard to call qsort with a null
array pointer, _even_ if you're asking it to sort zero elements so
that in principle it should never dereference that pointer. Fix the
four instances in PSCP/PSFTP where this was previously occurring.
Simon Tatham [Thu, 11 Jul 2013 17:24:32 +0000 (17:24 +0000)]
Fixes for the tree234 unit test: break its dependencies on half of the
rest of PuTTY, and fix a couple of format string type mismatches
pointed out by gcc.
Simon Tatham [Thu, 11 Jul 2013 17:24:28 +0000 (17:24 +0000)]
Add missing checks in update_for_intended_size() in the font selector
code, which would have coped badly if ever asked to select the first
font in the list at a size smaller than it supported. Luckily the
first font tended to be one of the X numeric aliases (e.g. 10x20)
which was stored with size zero, so this probably didn't actually come
up for anyone, but better safe than sorry.
Simon Tatham [Thu, 11 Jul 2013 17:24:20 +0000 (17:24 +0000)]
Add some missing null checks for inst->ldisc, which were causing
segfaults if a PuTTY or pterm did not close on exit and then you
either typed something via input_method_commit_event or changed the
line editing or echo settings.