The length coming back from ber_read_id_len might have overflowed, so
treat it as potentially negative. Also, while I'm here, accumulate it
inside ber_read_id_len as an unsigned, so as to avoid undefined
behaviour on integer overflow, and toint() it before return.
Thanks to Hanno Böck for spotting this, with the aid of AFL.
The initial test for a line ending with "PRIVATE KEY-----" failed to
take into account the possibility that the line might be shorter than
that. Fixed by introducing a new library function strendswith(), and
strstartswith() for good measure, and using that.
Thanks to Hanno Böck for spotting this, with the aid of AFL.
TOOLTYPE_NONNETWORK (i.e. pterm) already has "-log" (as does Unix
PuTTY), so there's no sense suppressing the synonym "-sessionlog".
Undocumented lacunae that remain:
plink accepts -sessionlog, but does nothing with it. Arguably it should.
puttytel accepts -sshlog/-sshrawlog (and happily logs e.g. Telnet
negotiation, as does PuTTY proper).
When we set ssh->sc{cipher,mac} to s->sc{cipher,mac}_tobe
conditionally, we should be conditionalising on the values we're
_reading_, not the ones we're about to overwrite.
Thanks to Colin Harrison for this patch.
I've added a few sample shell commands in the upload procedure (mostly
so that I don't have to faff about remembering how rsync trailing
slashes work every time), and also written a script called
'release.pl', which automates the updating of the version number in
all the various places it needs to be done and also ensures the PSCP
and Plink transcripts in the docs will match the release itself.
I spotted that I've been checking that old-style Windows Help files
were delivered with content-type "application/octet-stream", but not
also checking the same thing about the marginally newer .CHM ones. (Or
at least not writing it down in the wishlist; I think I did actually
check on at least one occasion.)
It's for regression testing and fuzzing, so there's no use for it if
you're not a developer working on the source.
Leaving it out of the 'make install' target in Makefile.gtk is no
trouble because that's already handled manually in Recipe by inserting
a giant hairy Makefile fragment to do the installation. But
Makefile.am was just setting bin_PROGRAMS to the full set of binaries
built, so for that one, I had to invent a new Recipe program category
[UT] which moves a particular binary into noinst_PROGRAMS.
While I was at it, I've retired the [M] program category, which has
been lying around unused since Ben's old Mac OS pre-X port.
Apparently if you maintain a branch for a long time where you only
compile with a non-default ifdef enabled, it becomes possible to not
notice a typo you left in the default branch :-)
This brings in the rest of the 0.66 branch, including some changes new
on master.
Conflicts:
doc/plink.but
sshrsa.c
(The conflicts were both trivial: in one, the addition of an extra
parameter to rsa2_newkey on master happened on the line next to 0.66's
addition of a check for NULL return value, and in the other, I'd got
the version number in the plink -h transcript messed up on master.)
Everything up to here on the release branch is cherry-picks from
master anyway, and some of their cherry-picked forms conflict with the
current state of master due to further work, so here I'm just
recording an ancestry relation to indicate that there's nothing up to
here on 0.66 that master hasn't got.
Handles managed by winhandl.c have a 'busy' flag, which is used to
mean two things: (a) is a subthread currently blocked on this handle
so various operations in the main thread have to be deferred until it
finishes? And (b) is this handle currently one that should be returned
to the main loop to be waited for?
For HT_INPUT and HT_OUTPUT, those things are either both true or both
false, so a single flag covering both of them is fine. But HT_FOREIGN
handles have the property that they should always be waited for in the
main loop, but no subthread is blocked on them. The latter means that
operations done on them in the main thread should not be deferred; the
only such operation is cleaning them up in handle_free().
handle_free() was failing to spot this, and was deferring freeing
HT_FOREIGN handles until their subthread terminated - which of course
never happened. As a result, when a named pipe server was closed, its
actual Windows event object got destroyed, but winhandl.c still kept
passing it back to the main thread, leading to a tight loop because
MsgWaitForMultipleObjects would return ERROR_INVALID_HANDLE and never
block.
(cherry picked from commit 431f8db862)
The aim is to try to reduce the incidence of the two least helpful
classes of those reports: the ones which have just got mismatched
checksum files, and the ones which don't tell us the information that
would help.
(cherry picked from commit 8ff3b22243)
This adds the "none" cipher and MAC, and also disables kex signure
verification and host-key checking. Since a client like this is
completely insecure, it also rewrites the client version string to
start "ISH", which should make it fail to interoperate with a real SSH
server. The server version string is still expected to begin "SSH" so
the real packet captures can be used against it.
The previous assertion failure is obviously wrong, but RFC 4253 doesn't
explicitly declare them to be a protocol error. Currently, the incoming
packet isn't logged, which might cause some confusion for log parsers.
Bug found with the help of afl-fuzz.
Starting up the random number generator is by far the slowest part of
plink's startup, and randomness is bad for fuzzing, so disabling it
should make fuzzing more effective.
This saves the need to fork and exec "cat", which should speed things
up. It also ensures that the network output goes to /dev/null, which
should avoid problems with blocking when writing to a full pipe.
It's not used outside logfopen, and leaving an invalid file pointer
lying around in the log context caused a segfault if the user
cancelled logging.
Bug found by afl-fuzz before it had even started fuzzing.
This is a minimal fix for CVE-2015-5309, and while it's probably
unnecessary now, it seems worth committing for defence in depth and to
give downstreams something reasonably non-intrusive to cherry-pick.
"amax == 0 || a[amax] != 0"
Essentially, when decodepoint_ed() clears the top bit of the key, it
needs to call bn_restore_invariant() in case that left the high-order
word zero.
Bug found with the help of afl-fuzz.
This one might be exploitable, since without the fix, ecdsa_freekey()
tries to wipe the bignum pointed to by an uninitialised pointer.
Bug found with the help of afl-fuzz.
Parameters are now accumulated in unsigned integers and carefully checked
for overflow (which is turned into saturation). Things that consume them
now have explicit range checks (again, saturating) to ensure that their
inputs are sane. This should make it much harder to cause overflow by
supplying ludicrously large numbers.
Fixes two bugs found with the help of afl-fuzz. One of them may be
exploitable and is CVE-2015-5309.
This is a minimal fix for CVE-2015-5309, and while it's probably
unnecessary now, it seems worth committing for defence in depth and to
give downstreams something reasonably non-intrusive to cherry-pick.
Parameters are now accumulated in unsigned integers and carefully checked
for overflow (which is turned into saturation). Things that consume them
now have explicit range checks (again, saturating) to ensure that their
inputs are sane. This should make it much harder to cause overflow by
supplying ludicrously large numbers.
Fixes two bugs found with the help of afl-fuzz. One of them may be
exploitable and is CVE-2015-5309.
The previous assertion failure is obviously wrong, but RFC 4253 doesn't
explicitly declare them to be a protocol error. Currently, the incoming
packet isn't logged, which might cause some confusion for log parsers.
Bug found with the help of afl-fuzz.
It's not used outside logfopen, and leaving an infalid file pointer
lying around in the log context caused a segfault if the user
cancelled logging.
Bug found by afl-fuzz before it had even started fuzzing.
This protects the Unix platform sharing code in the case where no salt
file exists yet in the connection-sharing directory, in which case
make_dirname() will want to create one by using some random bytes, and
prior to this commit, would fail an assertion because the random
number generator wasn't set up.
It would be neater to just return FALSE from ssh_test_for_upstream in
that situation - if there's no salt file, then no sharing socket can
be valid anyway - but that would involve doing more violence to the
code structure than I'm currently prepared to do for a minor elegance
gain.
