9fans - fans of the OS Plan 9 from Bell Labs
 help / color / mirror / Atom feed
From: Don Bailey <don.bailey@gmail.com>
To: Fans of the OS Plan 9 from Bell Labs <9fans@9fans.net>
Subject: Re: [9fans] lpdaemon
Date: Wed,  5 Jun 2013 07:54:50 -0600	[thread overview]
Message-ID: <F5E95FDE-7218-4C69-BB5E-8EE4FC90D5CC@gmail.com> (raw)
In-Reply-To: <51AF3F6A.8000605@gmx.de>

You're absolutely correct if the length of value to be copied is not validated prior to the copy. Then, an invalid page could be hit if no nil is present within the array or beyond.

I wasn't providing a verbatim patch (notice the function and operator weren't filled in). I was just providing the sequence of events that should occur. Eric points out correctly that strncpy effectively performs the first operation on the user's behalf. The second is achieved through the write to N.

To be verbose, my bypassing of strncpy is due to issues I've encountered in multi-threaded code. e.g. Don't trust libc copy functions in MT envs, always check post call.

An interesting and sometimes desirable effect of crashing on an invalid page read is that if memory can be corrupted, a consistent unexploitable crash is better than entering a context where the bug becomes exploitable.

D

On Jun 5, 2013, at 7:38 AM, Friedrich Psiorz <f.psiorz@gmx.de> wrote:

> I think your code is wrong. If the NUL byte is present, it doesn't do
> anything, however if it is not there, strlen will read more than it
> should, and possibly try to read some invalid address.
> In case info.host is a fixe size array, a simple
> info.host[sizeof info.host - 1] = 0;
> would do.
> 
> Am 05.06.2013 15:13, schrieb Don Bailey:
>> The first opportunity to write a nil byte should always be taken. Using sizeof only means that in corner cases memory disclosure may occur between where the nil should be and the end of the array. While this isn't a security critical app, it is still good coding practice.
>> 
>> x = strlen(info.host) < sizeof info.host ? strlen() : sizeof ;
>> info.host[x] = 0;
>> 
>> D
> 
> 



  reply	other threads:[~2013-06-05 13:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-05 11:38 yaroslav
2013-06-05 13:06 ` erik quanstrom
2013-06-05 13:13 ` Don Bailey
2013-06-05 13:20   ` erik quanstrom
2013-06-05 13:40     ` Don Bailey
2013-06-05 13:38   ` Friedrich Psiorz
2013-06-05 13:54     ` Don Bailey [this message]
2013-06-05 14:09       ` erik quanstrom
2013-06-05 14:29         ` Don Bailey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F5E95FDE-7218-4C69-BB5E-8EE4FC90D5CC@gmail.com \
    --to=don.bailey@gmail.com \
    --cc=9fans@9fans.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).