Skip to content

Commit 1e14e36

Browse files
Fix #8295 FN (error) Buffer is accessed out of bounds (wcpncpy, wcsncpy) (#4412)
* Fix #8295 FN (error) Buffer is accessed out of bounds (wcpncpy, wcsncpy) * Fix cfg, validation * Fix validation
1 parent df70436 commit 1e14e36

7 files changed

Lines changed: 48 additions & 9 deletions

File tree

cfg/cppcheck-cfg.rng

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@
283283
<attribute name="type">
284284
<choice>
285285
<value>strlen</value>
286-
<value>argvalue</value>
287286
<value>sizeof</value>
288287
<value>mul</value>
289288
</choice>
@@ -310,6 +309,24 @@
310309
<attribute name="baseType"><text/></attribute>
311310
</optional>
312311
</element>
312+
<element name="minsize">
313+
<attribute name="type">
314+
<choice>
315+
<value>argvalue</value>
316+
</choice>
317+
</attribute>
318+
<attribute name="arg">
319+
<ref name="ARGNO"/>
320+
</attribute>
321+
<optional>
322+
<attribute name="arg2">
323+
<ref name="ARGNO"/>
324+
</attribute>
325+
</optional>
326+
<optional>
327+
<attribute name="baseType"><text/></attribute>
328+
</optional>
329+
</element>
313330
</choice>
314331
</zeroOrMore>
315332
<optional>

cfg/posix.cfg

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5449,7 +5449,7 @@ The function 'mktemp' is considered to be dangerous due to race conditions and s
54495449
<not-overlapping-data ptr1-arg="1" ptr2-arg="2" size-arg="3"/>
54505450
<arg nr="1" direction="out">
54515451
<not-null/>
5452-
<minsize type="argvalue" arg="3"/>
5452+
<minsize type="argvalue" arg="3" baseType="wchar_t"/>
54535453
</arg>
54545454
<arg nr="2" direction="in">
54555455
<not-null/>

cfg/std.cfg

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5359,7 +5359,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun
53595359
<not-overlapping-data ptr1-arg="1" ptr2-arg="2" size-arg="3"/>
53605360
<arg nr="1">
53615361
<not-null/>
5362-
<minsize type="argvalue" arg="3"/>
5362+
<minsize type="argvalue" arg="3" baseType="wchar_t"/>
53635363
</arg>
53645364
<arg nr="2" direction="in">
53655365
<not-null/>

lib/checkbufferoverrun.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -593,10 +593,16 @@ static bool checkBufferSize(const Token *ftok, const Library::ArgumentChecks::Mi
593593
return Token::getStrLength(strtoken) < bufferSize;
594594
}
595595
break;
596-
case Library::ArgumentChecks::MinSize::Type::ARGVALUE:
597-
if (arg && arg->hasKnownIntValue())
598-
return arg->getKnownIntValue() <= bufferSize;
596+
case Library::ArgumentChecks::MinSize::Type::ARGVALUE: {
597+
if (arg && arg->hasKnownIntValue()) {
598+
MathLib::bigint myMinsize = arg->getKnownIntValue();
599+
unsigned int baseSize = tokenizer->sizeOfType(minsize.baseType);
600+
if (baseSize != 0)
601+
myMinsize *= baseSize;
602+
return myMinsize <= bufferSize;
603+
}
599604
break;
605+
}
600606
case Library::ArgumentChecks::MinSize::Type::SIZEOF:
601607
// TODO
602608
break;

lib/library.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -783,9 +783,6 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
783783
return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, valueattr);
784784
ac.minsizes.emplace_back(type, 0);
785785
ac.minsizes.back().value = minsizevalue;
786-
const char* baseTypeAttr = argnode->Attribute("baseType");
787-
if (baseTypeAttr)
788-
ac.minsizes.back().baseType = baseTypeAttr;
789786
} else {
790787
const char *argattr = argnode->Attribute("arg");
791788
if (!argattr)
@@ -804,6 +801,9 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co
804801
ac.minsizes.back().arg2 = arg2attr[0] - '0';
805802
}
806803
}
804+
const char* baseTypeAttr = argnode->Attribute("baseType"); // used by VALUE, ARGVALUE
805+
if (baseTypeAttr)
806+
ac.minsizes.back().baseType = baseTypeAttr;
807807
}
808808

809809
else if (argnodename == "iterator") {

test/cfg/posix.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,14 @@ size_t bufferAccessOutOfBounds_strnlen(const char *s, size_t maxlen)
564564
return len;
565565
}
566566

567+
void bufferAccessOutOfBounds_wcpncpy()
568+
{
569+
wchar_t s[16];
570+
wcpncpy(s, L"abc", 16);
571+
// cppcheck-suppress bufferAccessOutOfBounds
572+
wcpncpy(s, L"abc", 17);
573+
}
574+
567575
size_t nullPointer_strnlen(const char *s, size_t maxlen)
568576
{
569577
// No warning shall be shown:

test/cfg/std.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,14 @@ void bufferAccessOutOfBounds_wcsftime(wchar_t* ptr, size_t maxsize, const wchar_
556556
(void)wcsftime(ptr, maxsize, format, timeptr);
557557
}
558558

559+
void bufferAccessOutOfBounds_wcsncpy()
560+
{
561+
wchar_t s[16];
562+
wcsncpy(s, L"abc", 16);
563+
// cppcheck-suppress bufferAccessOutOfBounds
564+
wcsncpy(s, L"abc", 17);
565+
}
566+
559567
int nullPointer_wcsncmp(const wchar_t* s1, const wchar_t* s2, size_t n)
560568
{
561569
// cppcheck-suppress nullPointer

0 commit comments

Comments
 (0)