Skip to content

Fix/pre v2 bug fixes #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sparkeh9 merged 4 commits into from
Feb 28, 2026
Merged

Fix/pre v2 bug fixes #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f1186e
Use span-based read helper for control stream
sparkeh9 Jan 28, 2026
6dcb209
Override ReadByte for control stream
sparkeh9 Jan 28, 2026
18a2c2c
Fix issues #42, #34, #26 with tests
Feb 28, 2026
7ed6a3d
Fix PR 52 review feedback: vulnerabilities and CI issues
Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 51 additions & 49 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,51 @@
syntax: glob

### VisualStudio ###

# NodeJS
**/node_modules/*

# Tool Runtime Dir
/[Tt]ools/

# User-specific files
*.suo
*.user
*.userosscache
*.sln.docstates
*.xproj.user

# Build results
[Dd]ebug/
[Dd]ebugPublic/
[Rr]elease/
[Rr]eleases/
x64/
x86/
build/
bld/
[Bb]in/
[Oo]bj/
msbuild.log
msbuild.err
msbuild.wrn

# Cross building rootfs
cross/rootfs/

# Visual Studio 2015
.vs/

### OSX ###

.DS_Store
.AppleDouble
.LSOverride

/Source/packages/*
.vs
*DS_Store
project.lock.json

syntax: glob

### VisualStudio ###

# NodeJS
**/node_modules/*

# Tool Runtime Dir
/[Tt]ools/

# User-specific files
*.suo
*.user
*.userosscache
*.sln.docstates
*.xproj.user

# Build results
[Dd]ebug/
[Dd]ebugPublic/
[Rr]elease/
[Rr]eleases/
x64/
x86/
build/
bld/
[Bb]in/
[Oo]bj/
msbuild.log
msbuild.err
msbuild.wrn

# Cross building rootfs
cross/rootfs/

# Visual Studio 2015
.vs/

### OSX ###

.DS_Store
.AppleDouble
.LSOverride

/Source/packages/*
.vs
*DS_Store
project.lock.json

estorer_log.txt
build_log.txt
Expand Down
123 changes: 0 additions & 123 deletions build_log.txt
View file
Open in desktop

This file was deleted.

64 changes: 0 additions & 64 deletions restore_log.txt
View file
Open in desktop

This file was deleted.

53 changes: 46 additions & 7 deletions src/CoreFtp/FtpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,19 @@ public async Task LogOutAsync()
return;

Logger?.LogTrace( "[FtpClient] Logging out" );
await ControlStream.SendCommandAsync( FtpCommand.QUIT );
ControlStream.Disconnect();
IsAuthenticated = false;
try
{
await ControlStream.SendCommandAsync( FtpCommand.QUIT );
}
catch (Exception e)
{
Logger?.LogWarning( 0, e, "Error sending QUIT command during logout" );
}
finally
{
ControlStream.Disconnect();
IsAuthenticated = false;
}
}

/// <summary>
Expand All @@ -164,10 +174,27 @@ public async Task ChangeWorkingDirectoryAsync( string directory )

var pwdResponse = await ControlStream.SendCommandAsync( FtpCommand.PWD );

if ( !response.IsSuccess )
throw new FtpException( response.ResponseMessage );
if ( !pwdResponse.IsSuccess )
throw new FtpException( pwdResponse.ResponseMessage );

WorkingDirectory = ParseWorkingDirectory( pwdResponse.ResponseMessage );
}

WorkingDirectory = pwdResponse.ResponseMessage.Split( '"' )[ 1 ];
private string ParseWorkingDirectory(string responseMessage)
{
var parts = responseMessage.Split('"');
if (parts.Length >= 2)
{
return parts[1];
}

// Fallback for unquoted PWD responses like: /home/user is current directory
var firstSpace = responseMessage.IndexOf(' ');
if (firstSpace != -1)
{
return responseMessage.Substring(0, firstSpace);
}
return responseMessage;
}

/// <summary>
Expand Down Expand Up @@ -673,7 +700,19 @@ private async Task IgnoreStaleData()
public void Dispose()
{
Logger?.LogDebug( "Disposing of FtpClient" );
Task.WaitAny( LogOutAsync() );
try
{
if (IsConnected)
{
LogOutAsync().GetAwaiter().GetResult();
}
}
catch (Exception ex)
{
Logger?.LogWarning(0, ex, "Exception during logout on dispose.");
}

dataStream?.Dispose();
ControlStream?.Dispose();
dataSocketSemaphore?.Dispose();
}
Expand Down
12 changes: 12 additions & 0 deletions src/CoreFtp/Infrastructure/Stream/FtpControlStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ private int Read(Span<byte> buffer)
}




public override void Write(byte[] buffer, int offset, int count)
{
NetworkStream?.Write(buffer, offset, count);
Expand Down Expand Up @@ -190,6 +192,9 @@ public override void SetLength(long value)

protected async Task WriteLineAsync(string buf)
{
if (buf.Contains("\r") || buf.Contains("\n"))
throw new ArgumentException("Command parameters cannot contain CRLF characters", nameof(buf));

var data = Encoding.GetBytes($"{buf}\r\n");
await WriteAsync(data, 0, data.Length, CancellationToken.None);
}
Comment on lines +193 to 200
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The WriteLineAsync method appends \r\n to the provided buffer and sends it to the server without checking if the buffer itself contains CRLF sequences. This allows an attacker who can control the parameters of FTP commands (e.g., directory names, filenames) to inject arbitrary FTP commands into the stream. This is a classic CRLF injection vulnerability in the context of the FTP protocol.

To remediate this, sanitize all user-supplied inputs to remove or encode CRLF characters before they are used in FTP commands, or validate that the input does not contain these characters.

        protected async Task WriteLineAsync(string buf)
        {
            if (buf.Contains("\r") || buf.Contains("\n"))
                throw new ArgumentException("Command parameters cannot contain CRLF characters", nameof(buf));

            var data = Encoding.GetBytes($"{buf}\r\n");
            await WriteAsync(data, 0, data.Length, CancellationToken.None);
        }

Expand All @@ -205,12 +210,18 @@ protected string ReadLine(Encoding encoding, CancellationToken token)

token.ThrowIfCancellationRequested();

const int maxLineLength = 4096;

while (Read(buffer) > 0)
{
token.ThrowIfCancellationRequested();
data.Add(buffer[0]);
if ((char)buffer[0] != '\n')
{
if (data.Count > maxLineLength)
throw new FtpException("Line length limit exceeded");
continue;
}
line = encoding.GetString(data.ToArray()).Trim('\r', '\n');
break;
}
Expand Down Expand Up @@ -476,6 +487,7 @@ public void Disconnect()
}
finally
{
Socket?.Close();
Socket = null;
BaseStream = null;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/CoreFtp.Tests.Integration/CoreFtp.Tests.Integration.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<GenerateAssemblyConfigurationAttribute>false</GenerateAssemblyConfigurationAttribute>
<GenerateAssemblyCompanyAttribute>false</GenerateAssemblyCompanyAttribute>
<GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute>
<GenerateProgramFile>false</GenerateProgramFile>

</PropertyGroup>

<ItemGroup>
Expand Down
Loading