CWE 121 (Very High Severity security flaw) in tinyxml2.cpp #3687
Description
Veracode marks it as Very High severity flaw:
Copilot:
You're seeing CWE-121: Stack-based Buffer Overflow flagged because this code does an unchecked write into a buffer using memcpy, combined with a risky size cast and a pointer “back-up” trick that can easily go wrong.
Let’s break down the issues and then fix them cleanly.
What’s risky in this code?
void XMLPrinter::Write( const char* data, size_t size )
{
if ( _fp ) {
fwrite ( data , sizeof(char), size, _fp);
}
else {
char* p = _buffer.PushArr( static_cast<int>(size) ) - 1; // back up over the null terminator.
memcpy( p, data, size );
p[size] = 0;
}
}
- Unchecked memcpy write
memcpy(p, data, size) writes size bytes, and then p[size] = 0 writes one more byte.
If _buffer.PushArr(...) didn’t actually reserve size + 1 bytes, you’ll overwrite past the allocated memory → overflow (this is CWE-121/CWE-787).
- Pointer “back-up” (-1)
PushArr(int(size)) - 1 assumes PushArr appended a null terminator and that stepping back one byte lands you on valid, writable space. This is brittle and couples your write logic to undocumented _buffer internals. If PushArr changes, you now have a latent overflow.
- Dangerous size cast
static_cast(size) truncates on 64-bit systems if size > INT_MAX. That can allocate less than needed and then memcpy will overflow.
- Lack of basic guards
No null check for data.
No handling of size == 0.
No checking of fwrite result.