Bug 71219

Summary: Warn about (struct S*)malloc(n) where n < sizeof(struct S)
Product: gcc Reporter: Jonathan Wakely <redi>
Component: cAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: enhancement CC: msebor, muecker, sjames
Priority: P3 Keywords: diagnostic, patch
Version: 7.0   
Target Milestone: 14.0   
See Also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112347
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2023-09-19 00:00:00

Description Jonathan Wakely 2016-05-21 16:59:40 UTC
The change in http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2010 for the ISO/IEC TS 17961:2013 secure coding guidelines for C would make sense as a GCC warning, even if the rest of the TS isn't supported.

    struct S1 {
        unsigned int x;
        float        y;
        struct S1   *z;
    };


    struct S1 *f1(void) {
        struct S1 *p = (struct S1*)malloc(sizeof(p));  // diagnostic required
        return p;
    }

The malloc call can be presumed to be for an object of type struct S1, as implied by the cast and the variable it is used to initialize, so trying to allocate fewer than sizeof(struct S1) bytes should be diagnosed.

This would issue a warning for some of the bugs shown in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25702#c3 which are not diagnosed by -Wsizeof-pointer-memaccess
Comment 1 Martin Sebor 2016-05-26 15:33:12 UTC
I agree.  There's additional background on this rule in the CERT C Coding Standard guideline MEM35-C. Allocate sufficient memory for an object (https://www.securecoding.cert.org/confluence/x/2wE)

Let me add it to the of list security-related issues to diagnose I've been working on.
Comment 2 nsz 2016-05-26 16:06:53 UTC
note that casting the return value of malloc is an anti-pattern in c and dangerous (unfortunately widespread due to c++).

this effectively turns the type-checker off, which is an especially bad idea on a compiler that accepts implicitly declared function calls assuming int return type.
Comment 3 Jonathan Wakely 2016-05-26 17:52:52 UTC
That example is just taken straight from the WG14 document I linked to. Maybe the compiler should also be able to presume that the allocation is intended for struct S1 if you do:

        struct S1 *p = malloc(sizeof(p));

but I wanted to suggest following exactly what the secure coding guidelines require.
Comment 4 Jonathan Wakely 2020-11-10 17:31:15 UTC
Complete testcase:

#include <stdlib.h>

struct S1 {
  unsigned int x;
  float        y;
  struct S1   *z;
};


struct S1 *f1(void) {
  struct S1 *p = malloc(sizeof(p));  // diagnostic required
  return p;
}


It would probably make sense to not only warn for malloc, but also for other functions with __attribute__((malloc)) and __attribute__((alloc_size(n))) where n!=sizeof(*p). That would also help for xmalloc and similar wrappers in gcc and glibc.
Comment 6 GCC Commits 2023-11-01 21:37:51 UTC
The master branch has been updated by Martin Uecker <uecker@gcc.gnu.org>:

https://gcc.gnu.org/g:d880e093d92084f55b10626610ef059fd9194a6a

commit r14-5059-gd880e093d92084f55b10626610ef059fd9194a6a
Author: Martin Uecker <uecker@tugraz.at>
Date:   Thu Jul 27 13:36:05 2023 +0200

    c: Add Walloc-size to warn about insufficient size in allocations [PR71219]
    
    Add option Walloc-size that warns about allocations that have
    insufficient storage for the target type of the pointer the
    storage is assigned to. Added to Wextra.
    
            PR c/71219
    gcc:
            * doc/invoke.texi: Document -Walloc-size option.
    
    gcc/c-family:
    
            * c.opt (Walloc-size): New option.
    
    gcc/c:
            * c-typeck.cc (convert_for_assignment): Add warning.
    
    gcc/testsuite:
    
            * gcc.dg/Walloc-size-1.c: New test.
            * gcc.dg/Walloc-size-2.c: New test.
Comment 7 Sam James 2024-02-17 05:11:56 UTC
Fixed for 14?