[Bug middle-end/77608] missing protection on trivially detectable runtime buffer overflow

[Bug middle-end/77608] missing protection on trivially detectable runtime buffer overflow

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77608

Kees Cook <kees at outflux dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kees at outflux dot net

--- Comment #5 from Kees Cook <kees at outflux dot net> ---
I think this behavior may, unfortunately, be "as expected", due to how the
memcpy overflow checks are working (they're checking surrounding object, yes,
like bos(0) would)? The constant-expression bos() calculations do appear to
understand the base pointer object, but when faced with "i", it can't know for
sure -- it might have room (if "i" is < 3), or not. So it must return -1 as it
lacks any other context (like memcpy's "size" argument).

There may, however, be a missing opportunity for tightening the memcpy checker?

For example:


...
volatile unsigned i;

struct weird {
    char a[4];
    char b[8];
};

int main (void)
{
  {
    struct weird instance;
    char d [3];

    P (d + i);
    memcpy (d + i, "abcdef", 5); // always overflows d (the entire object)

    i = 7;
    P (instance.a + i); // can't see into "i"
    P (instance.a + 7); // room left in instance (5), but not "a" (0)

    memcpy (instance.a + i, "abcdef", 5); // misses a, doesn't overflow
instance. should this warn?

    __builtin_printf ("%.0s", d);
  }
}


-1 -1  0  0
-1 -1  0  0
 5  0  5  5

[Bug middle-end/77608] missing protection on trivially detectable runtime buffer overflow

[siddhesh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77608

Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |siddhesh at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |siddhesh at gcc dot gnu.org

--- Comment #6 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
The volatile causes it to bail out too early because we avoid operating on
trees with side effects, but at least without it __bos should be able to detect
undefined behaviour.

[Bug middle-end/77608] missing protection on trivially detectable runtime buffer overflow

[siddhesh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77608

Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #7 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
I've posted a patch:

https://gcc.gnu.org/pipermail/gcc-patches/2022-January/587698.html

which returns the whole size of an object (that's a thing now, since __bos
started handling negative offsets) if the offset is not a constant.  It goes on
top of the dynamic object sizes patchset.

Volatile offsets will need more rework (basically delay the side effects check
into tree-object-size), so I'll do that after all of these patches are through.

[Bug middle-end/77608] missing protection on trivially detectable runtime buffer overflow

[siddhesh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77608

--- Comment #8 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
The test case for pr 103961 exposed a flaw in my patch, where assuming
wholesize isn't always safe or at least would need more careful consideration. 
I need to think this through some more.