Bug 2773

Summary:

Potential segfault from ssh_remote_ipaddr()

Product:

Portable OpenSSH

Reporter:

Jakub Jelen <jjelen>

Component:

sshd

Assignee:

Assigned to nobody <unassigned-bugs>

Status:

CLOSED FIXED

Severity:

enhancement

CC:

djm

Priority:

Version:

7.5p1

Hardware:

Other

OS:

Linux

Bug Depends on:

Bug Blocks:

2852

Attachments:

Description	Flags
do not dereference NULL pointer before checking its validity	none

Description Jakub Jelen 2017-09-04 21:00:29 AEST

Created attachment 3050 [details]
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

Comment 1 Damien Miller 2017-09-05 10:13:47 AEST

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.

Comment 2 Jakub Jelen 2017-09-05 19:05:08 AEST

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.

Comment 3 Damien Miller 2018-06-01 14:05:49 AEST

similar fix applied; this will be in the 7.8 release

Comment 4 Damien Miller 2021-04-23 15:10:02 AEST

closing resolved bugs as of 8.6p1 release