Defect Report Summary for CSCR
Version 1.2

DR 1

DR 1 Prev <— Review —> Next DR 1, or summary at top

Submission Date: 2014-03-11
Source: WG14
Reference Document: N1801
Subject: error in 5.21 example

Summary

The (cut-down) requirement for rule 5.21 is:

A call to a standard memory allocation function is presumed to be intended for type T * when it appears in any of the following contexts

• in the right operand of an assignment to an object of type T *, or
• ...

The example for 5.21 is:


wchar_t *f1(void) {

    const wchar_t *p = L"Hello, World!";

    const size_t n = sizeof(p) * (wcslen(p) + 1);

    wchar_t *q = (wchar_t *)malloc(n); // diagnostic required

    /* ... */

    return q;

   }

Why is a diagnostic required on the malloc statement? On my machine

• T is wchar_t
• sizeof(p) == 4
• wcslen(p) == 13
• n == 56
• sizeof(wchar_t) == 2

Indeed, the only values of n that would require a diagnostic are 0 and 1.

I think the original intent was to make q smaller than 28 (the space required for 14 wide characters), then copy p to q (buffer overrun), but this rule doesn�t actually address that and the copy isn�t included in the example. That is, I think the original example was to be of the form:


wchar_t *f2(void) {

    const wchar_t *p = L"Hello, World!";

    const size_t n =  /*sizeof(p) * */ (wcslen(p) + 1); /* NB  n == 14  */

    wchar_t *q = (wchar_t *)malloc(n);

    wcscpy(q, p);

    return q;

   }

• buffer overrun is addressed elsewhere - though I cannot see where that is:
  • Annex C maps CWEs with Buffer Overrun to 5.22 � but that only talks about constructing an invalid pointer by pointer arithmetic or indexing � neither of which apply here
  • wcscpy may be regarded as a restricted sink, with a requirement that the size of the first parameter array >= the size of the second, so 5.46 could apply � except that 5.46 only applies to tainted data. The data in this example isn�t tainted
• It was recognised that it�s too difficult to carry the information around that lets you check what use is made of a buffer at an arbitrary point later in the program. The problem in the f2 example is on the line after the allocation, but in general it could be anywhere in the program

Suggested Technical Corrigendum

Either 5.21 needs a different example, such as :


struct S1 { int x, y, z;};   /*  sizeof(S1)  is 12  */



struct S1 *copyS1(const struct S1 s) {

    struct S1 *q = (struct S1 *)malloc(8); /*  Diagnostic required */

    *q = s;

    return q;

   }

I'd favour changing the example

Apr 2014 meeting

Committee Discussion

The words in 5.21 were revised but the example wasn't. A revised example was approved as a Proposed Technical Corregendum. A new paper was solicited to capture the issue raised by the changed example.

Oct 1014 meeting

Committee Discussion

A new paper N1860 proposing a new rule was submitted and discussed, and it was determined that since the original example illustrated an issue, that we could consider this a defect after all.

The following Proposed Technical Corrigendum corrects the existing example to match the 5.21 rule, and adds a new clause and example to capture the intent of the original 5.21 example.

The existing example has an explanation, and the words approved do not, and the following have been suggested.

EXAMPLE 1 In this noncompliant example, a diagnostic is required because n is sizeof(p), which is smaller than sizeof(struct S1), so N is 0

EXAMPLE 2 In this noncompliant example, a diagnostic is required because n is 14 and sizeof(wchar_t) is 2, making N equal 7. q is therefore treated as an array of 7 elements, but wcscpy attempts to copy p, an array with 14 elements, into it.

Proposed Technical Corrigendum

In rule 5.21, Rule section, replace

A call to a standard memory allocation function taking a size integer argument n and presumed to be intended for type T * shall be diagnosed when n < sizeof(T).

A call to a standard memory allocation function taking a size integer argument n and presumed to be intended for type T * shall be regarded as an array of N elements, where N = n / sizeof(T).

Any allocation where N == 0 shall be diagnosed (i.e. where n < sizeof(T)). Also, any attempt to use this array in a manner that causes its array bound to be violated shall be diagnosed.

EXAMPLE In this noncompliant example, a diagnostic is required because the value of n that is used in the malloc() call has been possibly miscalculated.

wchar_t *f1(void) {
  const wchar_t *p = L"Hello, World!";
  const size_t n = sizeof(p) * (wcslen(p) + 1);
  wchar_t *q = (wchar_t *)malloc(n);  // diagnostic required

  /* ... */
  return q;
}

EXAMPLE 1 



    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;

    }











EXAMPLE 2 



    wchar_t *f2(void) {

        const wchar_t *p = L"Hello, World!";

        const size_t n = (wcslen(p) + 1);

        wchar_t *q = (wchar_t *)malloc(n);

        wcscpy(q, p); // diagnostic required

        return q;

    }

DR 1 Prev <— Review —> Next DR 1, or summary at top