openssh bugs - Jun 2009 - [Bug 1606] New: internal-sftp does not drop conections properly, it will hang

https://bugzilla.mindrot.org/show_bug.cgi?id=1606

           Summary: internal-sftp does not drop conections properly, it
                    will hang
           Product: Portable OpenSSH
           Version: 5.2p1
          Platform: Itanium
        OS/Version: HP-UX
            Status: NEW
          Severity: normal
          Priority: P2
         Component: sshd
        AssignedTo: unassigned-bugs at mindrot.org
        ReportedBy: zpbrent at yahoo.com.cn


Created an attachment (id=1648)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1648)
Temporarily fix (changed code in session.c) for this issue

Hello: 

I have configured sshd_config as the following:
================================================Subsystem sftp
/opt/ssh/libexec/sftp-server
#Subsystem sftp internal-sftp
Match User sftpch
    ChrootDirectory /chrootdir
    ForceCommand internal-sftp
=================================================Now if somebody tries to do a
ssh (not a sftp) with account sftpch,
the connection hangs.

And I have investigated this issue and generated a temporarily solution
for it.

Let's have a travel to the ForceCommand section to execute
internal-sftp in do_exec() function, the source code just use
IS_INTERNAL_SFTP(command) to check whether sshd want to use
ForceCommand to execute internal-sftp or not and do not judge which
requirement from client, that means when our customers use ssh
(SUBSYSTEM_NONE) to connect to sshd but they have configured to use
ForceCommand to execute internal-sftp, the logic error will happened
(None Subsystem client connected to the internal-sftp Subsystem
server), it looks like this is the root cause about this hang.

Based on the root cause, I have generated a temporarily fix for this
issue. Just use IS_INTERNAL_SFTP(command) to check whether to set the
session->is_subsystem to SUBSYSTEM_INT_SFTP is not sufficient, we also
have use if (s->is_subsystem != SUBSYSTEM_NONE) to check if the
requirement from the client belong to subsystem or not meanwhile.

I will list the detial detail of the fix below and please help us to
have a review:
======================================================================In the
session.c source file
======================================================================845c845
+               if (s->is_subsystem && IS_INTERNAL_SFTP(command))
---
-               if (IS_INTERNAL_SFTP(command))
853c853
+               if (s->is_subsystem && IS_INTERNAL_SFTP(command))
---
-               if (IS_INTERNAL_SFTP(command))
======================================================================
With this temporarily fix, if somebody tries to do a ssh (not a sftp)
with account sftpch and sshd_config is set to above , the connection
will prompt 
"sh: internal-sftp:  not found 
Connection to <remote nodename> closed." 
and then drop conections properly instead of hang. And for sftp and ssh
with other account, it works correctly.

Are there anybody could help me to trace this issue and review my
temporarily fix since I am not an expert about Open-SSH code and I am
not confirm this fix dose not make other side effect for Open-SSH. If
you accept my fix, could you help me to port them into your next SSH
version, and if you think my fix is not considerable, could you help me
to figure out a new one? 

Best Regards
Brent
zpbrent at yahoo.com.cn

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1606


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org
             Blocks|                            |1626


-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1606



--- Comment #1 from Damien Miller <djm at mindrot.org> 2009-09-09 10:24:23
EST ---
*** Bug 1605 has been marked as a duplicate of this bug. ***

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1606

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1648|0                           |1
        is obsolete|                            |

--- Comment #2 from Damien Miller <djm at mindrot.org> 2009-10-23 12:19:31
EST ---
Created an attachment (id=1704)
 --> (https://bugzilla.mindrot.org/attachment.cgi?id=1704)
abort non-subsystem sessions to forced internal sftp-server

I think your proposed fix introduces a security vulnerability - a user
could upload a file "internal-sftp" and sshd would execute it, since
the SUBSYSTEM_INT_SFTP is no longer set when processing the session.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1606

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1704|                            |ok+
               Flag|                            |

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1606

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #3 from Damien Miller <djm at mindrot.org> 2009-11-20 10:42:22
EST ---
patch applied - this will be in openssh-5.4

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1606

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #4 from Darren Tucker <dtucker at zip.com.au> 2010-03-26
10:51:04 EST ---
With the release of 5.4p1, this bug is now considered closed.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.