Guard against overflow when allocating the null terminator

String deserialization requires one more byte than the length
computed from strlen. This CL guards against the case when
length+1 overflows.

Also include a slight optimization to memcpy string contents
instead of using std::copy. It's safe to cast away volatile
qualifiers here since the string data doesn't affect control flow.

Bug: dawn:680
Change-Id: Icaf319ea6c5aedcf0c33d17a0ea7c253f4f249e1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/41800
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This commit is contained in:
Austin Eng 2021-02-18 18:45:09 +00:00 committed by Commit Bot service account
parent 8c82acd778
commit b921b7dc9c
1 changed files with 10 additions and 1 deletions

View File

@ -330,12 +330,21 @@ namespace {
{% endif %}
{
size_t stringLength = transfer->{{memberName}}Strlen;
if (stringLength == std::numeric_limits<size_t>::max()) {
//* Cannot allocate space for the null terminator.
return DeserializeResult::FatalError;
}
const volatile char* stringInBuffer = nullptr;
DESERIALIZE_TRY(GetPtrFromBuffer(buffer, size, stringLength, &stringInBuffer));
char* copiedString = nullptr;
DESERIALIZE_TRY(GetSpace(allocator, stringLength + 1, &copiedString));
std::copy(stringInBuffer, stringInBuffer + stringLength, copiedString);
//* We can cast away the volatile qualifier because GetPtrFromBuffer already validated
//* that the range [stringInBuffer, stringInBuffer + stringLength) is valid.
//* memcpy may have an unknown access pattern, but this is fine since the string is only
//* data and won't affect control flow of this function.
memcpy(copiedString, const_cast<const char*>(stringInBuffer), stringLength);
copiedString[stringLength] = '\0';
record->{{memberName}} = copiedString;
}