full-disclosure - openssh remote exploit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20030916133748.GC27167@irccrew.org>
From: tss at iki.fi (Timo Sirainen)
Subject: openssh remote exploit

On Mon, Sep 15, 2003 at 04:31:56PM -0700, auto64746@...hmail.com wrote:
> you can see the 2 bugs in this code?, seems to of me that theo could
> not. i am of understanding that there are exploits working on this in
> the wild. 3 remote holes in default install now !

Since the patch is now available I'd like to rant a bit on it:

>         /* Increase the size of the buffer and retry. */
>         buffer->alloc += len + 32768;
>         if (buffer->alloc > 0xa00000)
>                 fatal("buffer_append_space: alloc %u not supported",
>                     buffer->alloc);

The problem was that if fatal() is called it may access the buffer again
with grown buffer->alloc but without actually having that much memory
allocated for it. The fix was to update buffer->alloc after the fatal()
check.

IMHO this is only a partial fix for the problem. Perhaps it's enough, but
it still feels a bit insecure.

I think there should be two more fixes (which may require large changes):

1) Input validation (buffer length checks) should be done way before calling
buffer.c so that the fatal() should be called only when there's a bug in
code. ie. the fatal() should be more like assert().

2) fatal() is insecure. There's tons of fatal() calls everywhere which in
turn call several cleanup functions. Are you sure there aren't any other
conditions where you call fatal() with inconsitent state?

I think the whole idea of cleanup functions is insecure. Log the error or
save it in memory and return failure from function. When you've came back to
the main loop, or a few other checkpoints where you can be sure the program
state is consistent, you'll do the cleanups and exit.

If something fatal happens and you want to quit immediately, make sure your
fatal handler is safe to be called _anywhere_ and don't call insecure
cleanups.