Defect | Summary | Date | Status |
---|---|---|---|
DR 1 | error in 5.21 example | 10/2016 | Published |
DR 2 | Missing formatted input/output functions in Rule 5.40 | 04/2017 | Closed |
DR 1 Prev <— Published —> 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
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).
{malloc is identified as a standard memory allocation function}
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
56 is not less than 2, so no diagnostic is required.
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:
This does have an issue - buffer overrun, but still doesn’t violate 5.21 as currently stated (14 is still >= 2).
I think the reason 5.21 got changed was either:
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;
}
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;
}
or the rule needs to be changed to reflect that the allocated memory is in effect an array of size n/sizeof(T),
and that it shouldn’t be indexed beyond that size (including by copy functions such as wcscpy etc. - this caveat may also need to be added to 5.22)
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 0EXAMPLE 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).with
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).In rule 5.21, replaceAny 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;
}
with
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 <— Published —> Next DR 1, or summary at top
DR 2 Prev <— Closed —> Next DR 2, or summary at top
Submitter: Clive Pygott
Submission Date: 2016-03-01
Source: WG14
Reference Document: N2006
Subject: Missing formatted input/output functions in Rule 5.40
Summary
This suggestion comes from MISRA, as they are adding support for 17961 to their rules.
Rule 5.40 names a number of functions that can attempt to write beyond the bounds of the target array, if supplied with tainted input, namely: fscanf, scanf, vfscanf, vscanf, sscanf, vsscanf and sprintf.
The observation is that vsprintf should be included in this list. Also the _s versions of all the above (including vsprintf_s) should be included, as they also can write beyond the end of the target array.
It is suggested that this is a defect rather than an enhancement, as from the rationale for the rule, they should have been included when drafted.
Apr 2016 meeting
Committee Discussion
The committee agrees with the author.
Proposed Technical Corrigendum
To 5.40 Rule section first sentence change:
Calls to thetofscanf
,scanf
,vfscanf
, andvsscanf
functions that pass...
Calls to theTo 5.40 Rule section second sentence change:fscanf
,scanf
,vfscanf
, andvsscanf
functions, and their Annex K counterpartsfscanf_s
,scanf_s
,vfscanf_s
, andvsscanf_s
, that pass...
Calls to thetosscanf
andvsscanf
functions
Calls to theTo 5.40 Rule section third sentence change:sscanf
,vsscanf
,sscanf_s
, andvsscanf_s
functions
Calls to the sprintf
function that
to
Calls to thesprintf
,vsprintf
,sprintf_s
, andvsprintf_s
functions that
DR 2 Prev <— Closed —> Next DR 2, or summary at top