openssh bugs - Sep 2017 - [Bug 2773] New: Potential segfault from ssh_remote_ipaddr()

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

            Bug ID: 2773
           Summary: Potential segfault from ssh_remote_ipaddr()
           Product: Portable OpenSSH
           Version: 7.5p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Created attachment 3050
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3050&action=edit
do not dereference NULL pointer before checking its validity

The function ssh_remote_ipaddr() is dereferencing ssh->state pointer
before it is checking it is not NULL.

This function is probably not called with closed connections in
upstream OpenSSH, but we are attaching audit messages explaining
reasons for failures and they can be used after the connection got
closed so we can see segfaults in some occasions.

For more information and debugging, see the RH bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=1488083

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
None of the packet layer functions work after the connection has been
freed.

I think the main problem here is that sshd is calling
audit_event(SSH_CONNECTION_ABANDON) and you seem to have patched
audit-linux.c to try to handle that case. You should handle it
separately, e.g. calling it early or skipping the ssh_remote_ipaddr()
call.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

--- Comment #2 from Jakub Jelen <jjelen at redhat.com> ---
Yes, it is a patched audit code, but I don't see a reason why the
packet layer could not return the cached values even after the
connection is cleaned up (this was one of the use cases why this data
is cached in the first place).

If it is a design decision, that it should not work, the code should
certainly not dereference NULL pointers before checking it and should
not segfault instead of gracefully failing (or falling back to UNKNOWN)
in this case, even though you are sure it can not be called from your
code without valid state.

We probably need the IP address for auditing reasons and I don't see a
simple way to call that earlier.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
similar fix applied; this will be in the 7.8 release


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2852
[Bug 2852] Tracking bug for OpenSSH 7.8 release
-- 
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=2773

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

-- 
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.