Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGXu5jLvFbfjYUuZnD0xK-9sgLbb-XPvTp1ieoTrMh_CNj0H-w@mail.gmail.com>
Date: Wed, 7 Dec 2016 14:08:40 -0800
From: Kees Cook <keescook@...omium.org>
To: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: Picking "Write a plugin to do format string
 warnings correctly"

On Tue, Dec 6, 2016 at 3:21 PM, Ruslan Kuprieiev <kupruser@...il.com> wrote:
> Hi!
>
> After watching a bunch of talks from Kees about security, I've finally
> decided to try to participate in KSPP.

Hi and welcome!

> If it's not taken, I would like to start with this task:
>
>      Write a plugin to do format string warnings correctly (gcc's
> -Wformat-security is bad about const strings)
>
> Unfortunately, I wasn't able to find any details about this task. Could
> someone provide some info about it, please?

Sorry those TODOs items are so terse; they're mostly just brain dumps
so we don't lose things. :)

The problem with -Wformat-security in gcc is that it doesn't correctly
notice a bunch of const string cases. If you look at my
format-security branch:
http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=format-security

There's a commit at the top called "format-security: move static
strings to const", which illustrates the first kind of problem gcc
has, which is not realizing that "const char *var" is "const char
var[]":

- const char *name = "fbcon";
+ const char name[] = "fbcon";

This can be seen with:

int main(int argc, char * argv[])
{
    char cmd[80];
    const char *pointer = "pointer: %p\n";

    snprintf(cmd, sizeof(cmd), "cat /proc/%d/maps", getpid());
    system(cmd);

    printf(pointer, pointer);
    fflush(NULL);
    pointer[0] = 'P';

    return 0;
}

$ gcc -Wall -Wformat=2 -Wformat-security format-security.c -o format-security
format-security.c: In function ‘main’:
format-security.c:22:5: warning: format not a string literal, argument
types not checked [-Wformat-nonliteral]
     printf(pointer, pointer);
     ^
format-security.c:24:16: error: assignment of read-only location ‘*pointer’
     pointer[0] = 'P';
                ^
And, this is right, if line 24 is removed, we can see the contents of
"pointer" are, as expected, in read-only memory:

$ ./format-security
55aa9d70b000-55aa9d70c000 r-xp 00000000 fd:06 28
  /tmp/format-security
55aa9d90b000-55aa9d90c000 r--p 00000000 fd:06 28
  /tmp/format-security
55aa9d90c000-55aa9d90d000 rw-p 00001000 fd:06 28
  /tmp/format-security
...
pointer: 0x55aa9d70b984

The reason gcc DOES warn about "const char *pointer" is because the
value of pointer itself can change. Building with "const char * const
pointer" tells gcc "pointer" should be read-only (though usually this
isn't enforced in variable placement). The goal would be to teach gcc
that if a variable is only ever assigned to read-only locations, it's
a false positive, so don't warn about it.

Here's an example from the kernel, in drivers/gpu/drm/ttm/ttm_memory.c:

static int ttm_mem_init_kernel_zone(struct ttm_mem_global *glob,
                                    const struct sysinfo *si)
{
        struct ttm_mem_zone *zone = kzalloc(sizeof(*zone), GFP_KERNEL);
...
        zone->name = "kernel";
...
        ret = kobject_init_and_add(
                &zone->kobj, &ttm_mem_zone_kobj_type, &glob->kobj, zone->name);


zone->name is pointing to a read-only string. -Wformat-security yells
about this because it can't tell that ->name is never reassigned
between the first place and where it gets used as a format string.


-Kees

-- 
Kees Cook
Nexus Security

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.