* Cast between closure types with a functional cast, not reinterpret_cast
A Borland __closure is a built-in (code, data) pair, so reinterpret_cast
between two closure types is allowed. Under standard C++ a closure is a
class, and reinterpret_cast cannot convert one class object into another.
The functional cast is the old C-style cast this line had before
"Refactoring old-style casts" (e5efed895), so C++Builder compiles it as it
did then, and a compiler implementing closures as classes can give it the
same meaning.
* Read the knocked FTP port's IPv4 address as 32 bits
The address is read through an unsigned long pointer. That is 32 bits on
Windows (LLP64), so it reads exactly the four address bytes there; on LP64
targets unsigned long is 64 bits, and the read runs four bytes past the
address into whatever follows it in gethostbyname's buffer. The assignment
to the 32-bit s_addr then keeps the right bytes on little-endian machines,
which is why it goes unnoticed, but the over-read is undefined. ULONG is 32
bits on every Windows compiler, as Windows API layers for POSIX define it
too.
* Compile the synchronize checklist's Explorer file operation on Windows only
TSynchronizeChecklistFileOperation carries out a synchronization checklist
through Explorer's IFileOperation (a Shell COM object), so that local
deletions go to the recycle bin with Explorer's progress and undo. Only
WinSCP's Windows GUI uses it (CustomScpExplorer.cpp). Elsewhere neither the
object nor the interfaces exist, and declaring them only to compile would
pretend to a facility that is not there; a frontend on another platform
implements the operation its own way. The class and its implementation are
guarded with _WIN32, which C++Builder defines, so the Windows build is
unchanged.
* Declare the SIOCATMARK result as ULONG, the type p_ioctlsocket takes
network.c declares ioctlsocket as taking ULONG * (its DECL_WINDOWS_FUNCTION)
but passes a u_long. Both are 32 bits on Windows, so the two agree there. On
an LP64 platform u_long is 64 bits and ULONG 32, so the pointer types differ
and the callee would write half the variable. ULONG is what the declaration
says, and is the same type on Windows.
* Pick aes-ni.c's _mm_setr_epi8 for any GCC-compatible compiler
MM_SETR_EPI8 uses _mm_setr_epi8 only under _WIN64, falling back to a brace
initialiser that is correct for MSVC's __m128i (a byte union) but fills
clang's and GCC's (a two-lane vector) wrongly; the _WIN64 condition fixed
that for Win64 builds only. Any compiler defining __GNUC__ (clang does) now
takes the intrinsic.
* Compile WinSCP's Windows-only additions to neon on Windows only
WinSCP builds neon for Win32 alone, and three of its additions assume it:
ne_openssl.c includes <windows.h> (it uses nothing from it), timed_connect
switches the socket to non-blocking mode with Winsock's ioctlsocket instead
of upstream's fcntl, and ne_sock_sspi_init calls into SSPI unconditionally.
On POSIX neon uses BSD sockets directly, so the include is guarded with
_WIN32, the two ioctlsocket branches are taken only on _WIN32 (upstream's
fcntl code beside them is used otherwise), and ne_sock_sspi_init calls SSPI
under HAVE_SSPI, as ne_sock_init's own call does, reporting failure (-1)
without it -- which WebDAVFileSystem.cpp already handles as SSPI being
unavailable. C++Builder defines _WIN32 and neon's Win32 config.h defines
HAVE_SSPI, so the Windows build is unchanged.
* Use in_addr's s_addr in FileZilla's proxy layer
AsyncProxySocketLayer.cpp reaches the IPv4 address through Winsock's union,
sin_addr.S_un.S_addr. Winsock also defines s_addr as exactly that member
(#define s_addr S_un.S_addr), and s_addr is the name BSD sockets and POSIX
give the field; a POSIX in_addr has no S_un. Same field, so the Windows
build is unchanged.
* Pass FileZilla's two tool strings as UnicodeString, not std::wstring
CFileZillaTools::GetClientString and CustomReason return std::wstring; the
rest of the interface between core and FileZilla, and both sides' own code,
use UnicodeString or wchar_t pointers. Built with a 2-byte wchar_t on a
platform whose C++ library assumes 4 (clang's -fshort-wchar on Linux),
std::wstring is unusable: libstdc++ provides basic_string<wchar_t> as a
precompiled extern template for its own wchar_t, and char_traits<wchar_t>
calls the C library's 4-byte wcslen and wmemcpy: a Linux build of
FtpFileSystem.cpp imported wcslen through it. The values are only built from
UnicodeString, compared with empty() and read with c_str(), so UnicodeString
is a direct replacement (IsEmpty for empty). On Windows, where wchar_t is 2
bytes throughout, behaviour is unchanged.
* Build storage key paths with backslashes on every platform
THierarchicalStorage joins registry and INI key paths (Sessions\My site)
through its IncludeTrailingBackslash and ExcludeTrailingBackslash members,
which defer to the RTL functions of those names. The RTL's add and strip the
platform's path delimiter: a backslash on Windows, a slash on POSIX, where
these functions also build local file paths. Keys are not file paths: their
separator is a backslash everywhere, and the INI file's sections say so. The
members now add and strip a backslash themselves; on Windows that is what
the RTL did, so the behaviour there is unchanged. Without this, on POSIX the
current key became "Sessions/", matched no section, and no stored site
loaded.
* Hand neon a copy of the TLS-init closure, not its two pointers
SetNeonTlsInit stores the closure in the neon session by reinterpreting it
as a TMethod and saving its code and data pointers, and ne_init_ssl_session
rebuilds it with MakeMethod. That relies on a closure being exactly those
two pointers, which only Borland's __closure is; with any other closure type
the saved words are its internals, read from a by-value parameter that is
gone by the time neon calls back, and the rebuilt closure is garbage (a
clang build's first WebDAV-over-TLS connection crashed there). The session
now holds a heap copy of the closure, freed by neon's destroy-session hook
-- which also covers the sessions libs3 creates and destroys itself -- and
ne_init_ssl_session calls that copy. On C++Builder the behaviour is the
same, at the cost of one small allocation per session.
* Take CString's search results as const wchar_t *
CString::Find, ReverseFind and FindOneOf search the const string c_str()
returns and keep the result in a wchar_t *. ISO C++ declares wcschr,
wcsrchr, wcsstr and wcspbrk on a const string as returning const wchar_t *,
as MSVC's and newer mingw-w64 headers (llvm-mingw's) do, so the
initialisation does not compile there. The pointers are only compared and
subtracted: const is all they need.
* Index the string a string-list property returns, not the property
GetLocales takes a character of Exts->Strings[Index]. Under clang the
__declspec(property) standing for TStrings::Strings' __property reads a
second subscript as the property's own second index, so the expression is a
two-argument call to the getter and does not compile (parentheses do not
change that). Read into a UnicodeString first, the second subscript indexes
the string the property returns, as under C++Builder.
---------
Co-authored-by: Eduardo Correia <ecorreia@apliant.com.br>
* Translate old-format RSA host keys in retrieve_host_key
retrieve_host_key comes from PuTTY's check_stored_host_key, where key is the
server's key and an old-format registry entry is translated and kept only if
it matches. In retrieve_host_key key is the output buffer, so the
translation is compared with whatever the caller's buffer holds (TSecureShell
passes an uninitialised one) and in practice never matches: an old-format
host key is reported as not cached.
The translation is now returned as the stored key, for the caller to compare
with the server's key, as it compares every other stored key. It is not
written back in the new format, as nothing here has verified it.
* Keep the original strbuf_to_str in retrieve_host_key's old-format path
Exclude only the comparison, the put_reg_sz and the else (#ifndef WINSCP),
so that it is clear the strbuf_to_str is what the original does when the
key matches.
---------
Co-authored-by: Eduardo Correia <ecorreia@apliant.com.br>
ec_cleanup frees every curve ecc-ssh.c sets up except Ed448: ec_ed448 lacks
the WINSCP_CURVE_CLEANUP its siblings have and ec_cleanup does not call it,
so its points and bignums leak at shutdown once an Ed448 key was used.
win_misc_cleanup frees get_system_dir's cached buffer but leaves the pointer
and its size, so a get_system_dir call after the cleanup returns freed
memory instead of fetching the directory again.
random_add_noise enters putty_section and then returns without leaving it
when the generator is not active. The section is recursive, so the thread
that returned keeps working, but any other thread entering it -- another
session's socket, RNG or bignum code -- then blocks for good.
aes-ni.c pushes its target attribute under #ifdef __clang__ but pops it
under #ifdef _WIN64. For clang targeting Win64 the two coincide. Otherwise
they do not: clang targeting 32-bit Windows (clang-cl in a WINSCP_VS build)
reports the push as unterminated and fails, and MSVC targeting x64 meets a
pop of a clang pragma it does not know (warning C4068). The pop now has the
push's condition.
This prevents a couple of different possible crashes if a PPK file
constructed by something other than PuTTYgen contains invalid
parameters. If parallelism is 0 then the Argon2 code can divide by
zero; if the memory requirement is too small for the parallelism then
it can overrun a buffer.
Signed-off-by: Martin Prikryl <martin@winscp.net>
Source commit: e66db8e4bd600fd9d92467fb8b6cc31cd86fc0ad
This case led to a tight loop in ssh2_try_send, which would calculate
the largest size of data packet it could send on a channel as the min
of various things including the max packet size; find, of course, that
the largest size was therefore 0; send no data; and loop round again
to keep trying.
There's nothing you can sensibly do about that by the time you get to
ssh2_try_send, so a better response is to reject the connection at
open time.
When the connection is initiated from the far end (i.e. we receive
CHANNEL_OPEN) we can do this relatively nondestructively, by rejecting
just that one channel-open request. When it's initiated from our end,
and the CHANNEL_OPEN_CONFIRMATION we got back contained the zero
maxpkt, there's no similar protocol method for rejecting the
confirmation, so instead we disconnect the whole SSH session.
(That seems harsh, but I think it's reasonable, because one reason for
aborting an SSH connection is that you genuinely don't _know_ what
your client can legally do next – and this is exactly that situation.)
Signed-off-by: Martin Prikryl <martin@winscp.net>
Source commit: 5b7de97f5fcb3f4b957e9585ea15018831c8b547
The space allocated for the decrypted packet was 4 bytes too small,
because we end up reading (s->packetlen + s->maclen) bytes into it,
but we allocated the buffer based on the largest possible size of
(s->len + s->maclen). The two differ by 4. So if a packet is within 4
bytes of the largest size accepted by the check of s->len against
OUR_V2_PACKETLIMIT, then it overruns the buffer allocated for the
following BPP_READ.
Thanks to Yukihiro Nakamura for spotting this.
Signed-off-by: Martin Prikryl <martin@winscp.net>
Source commit: dc1ea030899bc7ec7ed4388cfc01757c41816f0c
And adding it to a project file for consistency and 64-bit compatibility (for which it will be compiled by bcc64)
Source commit: edc84e36955a053c777b9ebedca5b6a354dc202e
Primarily because linking MSVC object files to Clang executable do not seem to be possible anymore.
But there's even 25% boost comparing to MSVC for SHA-256 and AES SW (Argon not measured)
AES NI won't compile without huge changes (alignment, partial reimplementation in asm), which hopefully won't be needed on 64-bit (and inline asm is allegedly not even possible with 64-bit Clang).
So sticking with MSVC for now.
With the changes similar boost as with Clang. Though -O3 optimization (BCC_OptimizeMaximum) was needed. Without it, the speed was 10x slower.
Source commit: 07d16b2a5ebf3b166af572a76fdbab6c488cc00f
Most, if not all, were for compatibility with the classic C++Builder compiler (declarations in the middle of code + inline structure initializers)
Source commit: a9f38c3a07777e538c3c6e0fbdd38e9d68327dfa
with aim to eventually update the code to need as little warnings disabled as needed.
Some code updates (and even fixes) to avoid warning done already
Source commit: 5c45a6f56c52e7b0c5034ed9980cd65259a7372b
Mostly:
– incorrectly identified uninitialized variables
– explicit static global variables
– const, sign and down casts and safety
– typename use
– semicolon after function implementation
– unused variables, fields, arguments and macros
– field initialization order
– exact include filename case
– redundant in code macros already defined at project level
(cherry picked from commit f7f00c631e)
Source commit: 90ad9390853aba308abe73cf02e0281238c53c87
Notably:
Bug fix: Monospace font is not remembered if SSH banners are administratively forced
Change: Bug fix: In XML log, filename tag was included in stat tag, although explicitly documented not to
(cherry picked from commit 4019b56ffb)
Source commit: 8f8d10bc0c9f4ca626dceda1d3e9889dc22f4394
