On Dec 15, 2016, at 4:38 AM, Jakob Schlyter <jakob at kirei.se>
wrote:> On 2016-12-15 at 01:05, Darren Tucker wrote:
>
>> On Thu, Dec 15, 2016 at 6:58 AM, Blumenthal, Uri - 0553 - MITLL
>> <uri at ll.mit.edu> wrote:
>> [OSX launchd diff]
>>> I for one would like to see it merged.
>>
>> I took the patch and addressed the comments in
>> https://bugzilla.mindrot.org/show_bug.cgi?id=2341. If we can get some
>> confirmation that it
>> (https://bugzilla.mindrot.org/attachment.cgi?id=2915&action=edit)
>> works OK we might be able to get that in.
>
> People running macOS, please test this patch by doing X11 forwarding with
macOS client to any server. To get current code easiser, use branch
'osx' from repo https://github.com/jschlyter/openssh-portable.git.
Looking at this patch, it seems to me that it introduces a possible exploit. The
new code calls stat() on whatever string is set as the display value, even
before checking for display values that are meant to refer to remote network
hosts. If ?ssh? is run in a directory which happens to have a file/pipe/socket
named to match one of those network display values, this new code would return
that it should connect to this local socket rather than the remote host when
doing the forwarding.
While checking for ?/tmp/launch? as a prefix is a problem now that MacOS is
putting these local sockets in paths starting with
?/private/tmp/com.apple.launchd?, I think this new code should at a minimum
require that the path start with a leading ?/? before treating it as a local
socket and doing a stat() on it.
--
Ron Frederick
ronf at timeheart.net