An E record received before any D record made the directory depth
counter negative and moved the current directory to the parent of the
destination, so that a following C record wrote outside it; each further
E record climbed another level. On Windows, a D record named "...", ".. "
or " .." (legal names on a Unix server) normalizes to the current
directory, so a D and E pair climbed one level with a balanced counter.
An unmatched E record is now rejected before it is acknowledged. Every C
and D record name goes through ResolveChild, which requires the full path
to be a direct child of the current directory with exactly the name that
was sent. This also rejects Windows device names such as "CON", and names
that Windows would silently change, such as a trailing dot, unless the
destination uses the \\?\ prefix. Rejected names are reported to the
server and thrown as ScpException. ResolveChild replaces
EnsureValidLocalName.
The C, D and T record patterns were not anchored and D was tried before
C, so a file named, for example, "zD0755 0 x" was treated as a directory
and its content read as SCP records, letting whoever controls a file on
the server choose the names and contents of files written locally.
The patterns now match the whole line. An invalid or unexpected record is
reported to the server and thrown as ScpException; previously an unknown
record was reported and ignored, and Download(string, Stream) then returned
normally with an empty stream.
Download(string, FileInfo) no longer shares the directory loop. It
accepts an optional T record followed by a C record, so a D or E record
can no longer cause an InvalidCastException or let later C records
overwrite the file. The directory loop moves to DownloadDirectoryContent
and the content copy is renamed DownloadFileContent.
The file name part of the path passed to Upload(Stream, string) and
Upload(FileInfo, string) is written verbatim into the C record, as are
local file and directory names in Upload(DirectoryInfo, string). A line
feed in such a name ends the record early, so the remainder of the name
and the file content can be interpreted as further SCP commands in the
byte stream, for example creating additional files in the target directory.
Throw ArgumentException in Upload(Stream, string) and Upload(FileInfo, string)
for this case, and ScpException in Upload(DirectoryInfo, string) when it comes
across such a local file or directory. Also reject paths without a file name
part.
A malicious or compromised SCP server could return file or directory names containing
path separators, drive qualifiers, or parent-directory references.
ScpClient.Download(string, DirectoryInfo) combined these into a local path without
validation, allowing writes outside the destination directory. Server-supplied C and D
record names are now validated before being combined into a local path.
Signed-off-by: Nadav0077 <18245584+Nadav0077@users.noreply.github.com>
SCP performs a transfer by running scp on the server with the remote path
embedded in a command. On a shell-based server that command is interpreted
by a shell, so a path that is not quoted to suit that shell can be executed
as a command on the server (GHSA-mggc-4xg6-vcxf); on a non-shell-based
server the path is used literally and must not be quoted at all. The right
encoding therefore depends on the server, and no single transformation is
safe for every server.
Rather than default this choice, obsolete the ScpClient constructors that
implicitly used DoubleQuote and add constructors that take an
IRemotePathTransformation explicitly, so callers must choose one suited to
their server and trust environment. DoubleQuote remains the default for the
obsolete constructors, so existing behaviour is unchanged. Document the
consideration on ScpClient and IRemotePathTransformation, and recommend
using SFTP.
* Bump the dependencies group with 4 updates
Bumps BouncyCastle.Cryptography from 2.6.1 to 2.6.2
Bumps Meziantou.Analyzer from 2.0.205 to 2.0.210
Bumps MSTest from 3.9.3 to 3.10.0
Bumps SonarAnalyzer.CSharp from 10.13.0.120203 to 10.15.0.120848
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Fixes spelling errors in comments, private members, and public members in test projects
* Adds en-GB as spell checker option; reverts notable cases of American English
* convert file UTF-16 LE BOM -> UTF-8
---------
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
* Preparation to enforce formatting and style in CI
- enabled IDE0055 to enforce formatting on build
- disabled SA1137 and SA1025 because they are already
covered by IDE0055
- disabled SA1021 because it conflicts with csharp_space_after_cast
* Cleanup formatting and style on codebase
This commit has no manual changes, it is the result
of running "dotnet format whitespace" and "dotnet format style"
* new formatting fixes after merge
* appveyor: set git autocrlf
as suggested by sharwell to hopefully fix Windows CI.
* use autocrlf input
to hopefully fix Linux tests
* fix formatting
* use autocrlf input for Linux only
* set csharp_space_after_cast to false
* Revert SA1021 suppression
---------
Co-authored-by: Robert Hague <rh@johnstreetcapital.com>
* Move test projects to test folder.
Move global.json to root of repo.
Update solution items in solution.
* Move test projects to test folder.
Move global.json to root of repo.
Update solution items in solution.
Update appveyor configuration/
* Attempt to have appveyor use the correct .NET SDK.
* Update .NET SDK to version 7.0.402.
* Move Data folder below Renci.SshNet.Tests.
* Make csinst less chatty.
* Move Data folder directly below test folder as it's used by multiple test projects.
* Remove CS1591 nowarn from concrete test projects as this is already defined in the Directory.Build.props that is in the test folder.
* Fix integration test after moving test projects
---------
Co-authored-by: drieseng <gert.driesen@telenet.be>
Rob Hague