Servers that do not implement RFC 4252 correctly may lead to stack overflow

[shahsumit]

We are using SSH.NET(2014.4.6.0) dll to connect to the sftp sites in our application. If all the required parameters are correct the code works as desired. But if the sftp password is incorrect the system crashes. The try catch block is not able to catch the error. An log is written in the event viewer “The process was terminated due to stack overflow.”

Added the reference of the latest Renci dll from package manager console. Have tried using the beta version of the dll as well with no success(https://www.nuget.org/packages/SSH.NET)

Debugged the code when the sftp password is incorrect and below are the findings:

• The authenticationMethod.Authenticate(session) always returns authentication result as "PartialSuccess"
• Because of this the "TryAuthenticate" methods goes into an recursive loop
• We are getting the stack overflow exception and the system crashes.

This being an production issue, we have deployed a quick fix.

• In the PasswordAuthenticationMethod.cs class file, commented the below 3 lines of code in the "Session_UserAuthenticationFailureReceived" method:
  //if (e.Message.PartialSuccess)
  // _authenticationResult = AuthenticationResult.PartialSuccess;
  //else
• Doing this the authentication result returned for the Password method will either be success or failure.
• Also caught the "SshAuthenticationException" exception.

Note: This is not happening all the time but for certain sftp sites.

Requesting you to please look into this issue.

[darkoperator]

You are using a very old version of the library. There are more recent version with many bug fixes. Have you tried those?

…

Sent from my iPhone

On Sep 26, 2017, at 8:27 AM, shahsumit ***@***.***> wrote: We are using SSH.NET(2014.4.6.0) dll to connect to the sftp sites in our application. If all the required parameters are correct the code works as desired. But if the sftp password is incorrect the system crashes. The try catch block is not able to catch the error. An log is written in the event viewer “The process was terminated due to stack overflow.” Added the reference of the latest Renci dll from package manager console. Have tried using the beta version of the dll as well with no success(https://www.nuget.org/packages/SSH.NET) Debugged the code when the sftp password is incorrect and below are the findings: The authenticationMethod.Authenticate(session) always returns authentication result as "PartialSuccess" Because of this the "TryAuthenticate" methods goes into an recursive loop We are getting the stack overflow exception and the system crashes. This being an production issue, we have deployed a quick fix. In the PasswordAuthenticationMethod.cs class file, commented the below 3 lines of code in the "Session_UserAuthenticationFailureReceived" method: //if (e.Message.PartialSuccess) // _authenticationResult = AuthenticationResult.PartialSuccess; //else Doing this the authentication result returned for the Password method will either be success or failure. Also caught the "SshAuthenticationException" exception. Note: This is not happening all the time but for certain sftp sites. Requesting you to please look into this issue. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[jbpatelgit]

Yes. We downloaded latest beta3 and tried. It still has the issue.

[darkoperator]

When you debug the app in visul studio where does it say the failure is? In C# in my apps I have not seen that error and try catch for the error types seem to work (not the author simply a user) From: jbpatelgit Sent: Tuesday, September 26, 2017 9:26 AM To: sshnet/SSH.NET Cc: Carlos Perez; Comment Subject: Re: [sshnet/SSH.NET] Sftp Client: Stack overflow exception in case ofincorrect password. (#306) Yes. We downloaded latest beta3 and tried. It still has the issue. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[jbpatelgit]

Try.Catch doesn't catch stack overflow errors. It just crashes the process, which is a windows service in our case.

[drieseng]

@jbpatelgit I think the stack overflow happens when the SSH server expects two factor authentication with 2x password, and the password is not correct. I'll try to reproduce it tomorrow.

[drieseng]

@shahsumit Would it be possible for you to build SSH.NET from source? In the case, I could send a modified version of our ClientAuthentication class with extra tracing.

I haven't yet managed to reproduce this issue.

[jbpatelgit]

Sure. Please send us the class and details on how to enable the trace.

[drieseng]

I replied to your initial email.

[shahsumit]

Replaced the ClientAuthentication.cs class with the one emailed. Attached is the console log. Basically the authentication result being returned is "PartialSuccess" which again triggers the TryAuthenticate method. This goes into an recursive loop which lastly gives StackOverflow exception.

SftpErrorLog.txt

[drieseng]

@shahsumit I'm working on a fix for the stack overflow.

The underlying cause is bad behavior from the SSH server. If any given authentication request fails, it should respond with a SSH_MSG_USERAUTH_FAILURE message with 'partial success' set to FALSE. The server you're using appears to set the value of 'partial success' to TRUE for an authentication request that fails (due to wrong password).

From RFC 4252:

The value of 'partial success' MUST be TRUE if the authentication request to which this is a response was successful. It MUST be FALSE if the request was not successfully processed.

I will guard against a SSH server that does not respect this RFC by introducing a limit for the number of times we attempt to authenticate using a given method for which we received a 'partial success' response.

Nonethless, I would still like to report this as an issue against this SSH server. What SSH server (and version) are you using?

[jbpatelgit]

The server is of one of our clients. We have no idea what it is but surely will report the issue. Will you putting the fix soon to restrict the # of retries? - Jayesh

…

________________________________ From: Gert Driesen <notifications@github.com> Sent: Sunday, October 1, 2017 8:18 PM To: sshnet/SSH.NET Cc: jbpatelgit; Mention Subject: Re: [sshnet/SSH.NET] Sftp Client: Stack overflow exception in case of incorrect password. (#306) @shahsumit<https://github.com/shahsumit> I'm working on a fix for the stack overflow. The underlying cause is bad behavior from the SSH server. If any given authentication request fails, it should respond with a SSH_MSG_USERAUTH_FAILURE message with 'partial success' set to FALSE. The server you're using appears to set the value of 'partial success' to TRUE for an authentication request that fails (due to wrong password). From RFC 4252<https://www.ietf.org/rfc/rfc4252.txt>: The value of 'partial success' MUST be TRUE if the authentication request to which this is a response was successful. It MUST be FALSE if the request was not successfully processed. I will guard against a SSH server that does not respect this RFC by introducing a limit for the number of times we attempt to authenticate using a given method for which we received a 'partial success' response. Nonethless, I would still like to report this as an issue against this SSH server. What SSH server (and version) are you using? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#306 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/Aezfx_9M5GPCD0mQAyxdo4AJq-vepZdyks5sn_OXgaJpZM4PkKPJ>.

[drieseng]

@jbpatelgit I'm working & commuting between 11 and 14 hours a day, and have 2 kids and a gf that don't nearly get the attention they deserve. Let's just say that I'm going as fast as I can.

The stack overflow issue is fixed locally, and I'm mostly working on adding unit tests to avoid future regressions.

Can you use these instructions to get me the name and version of the SSH software that your client is using?

[jbpatelgit]

Gert,

I never meant to rush you in anyway. I was just asking for the status. Take all the time you need as we have put a work around for now.

I will get you the server version tomorrow.

Thank you very much for all efforts to keep this nice open source library alive.

[drieseng]

never meant to rush you in anyway. I was just asking for the status. Take all the time you need as we have put a work around for now.

I know. There are just days where it gets a little too much.
I'll try to commit the fix tomorrow, and release a new beta this weekend.

[shahsumit]

Hi Gert,

Please find below, the Sftp server version and name:

SSH-2.0-1.82_sshlib Globalscape

Please let us know if any more information is required.

[drieseng]

@shahsumit Thanks for the info. I'll look into that server later.

@jbpatelgit I created PR #311 for the fix. It's not yet complete, but should work for you.

[drieseng]

@jbpatelgit I've merged the fix in the develop branch.
Let me know if it fixes the problem for you.

[shahsumit]

Hi Gert,

We downloaded the code from the develop branch and the fix is working for us now. Could you please let us know when will this be released so that we can incorporate the new dll in our production environment.

[drieseng]

Beta 4 of SSH.NET version 2016.1.0 is now available on GitHub and via NuGet.