Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions src/lib/support/Span.h
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,12 @@ inline CHIP_ERROR CopySpanToMutableSpan(ByteSpan span_to_copy, MutableByteSpan &
{
VerifyOrReturnError(out_buf.size() >= span_to_copy.size(), CHIP_ERROR_BUFFER_TOO_SMALL);

// There is no guarantee that span_to_copy and out_buf don't overlap, so use memmove()
memmove(out_buf.data(), span_to_copy.data(), span_to_copy.size());
// No guarantee that span_to_copy and out_buf don't overlap, so memmove() not memcpy().
// An empty span may have a null data(), it is undefined behaviour to pass it to memmove() even at zero length.
if (!span_to_copy.empty())
{
memmove(out_buf.data(), span_to_copy.data(), span_to_copy.size());
}
out_buf.reduce_size(span_to_copy.size());

return CHIP_NO_ERROR;
Expand All @@ -413,8 +417,12 @@ inline CHIP_ERROR CopyCharSpanToMutableCharSpan(CharSpan cspan_to_copy, MutableC
{
VerifyOrReturnError(out_buf.size() >= cspan_to_copy.size(), CHIP_ERROR_BUFFER_TOO_SMALL);

// There is no guarantee that cspan_to_copy and out_buf don't overlap, so use memmove()
memmove(out_buf.data(), cspan_to_copy.data(), cspan_to_copy.size());
// No guarantee that cspan_to_copy and out_buf don't overlap, so memmove() not memcpy().
// An empty span may have a null data(), it is undefined behaviour to pass it to memmove() even at zero length.
if (!cspan_to_copy.empty())
{
memmove(out_buf.data(), cspan_to_copy.data(), cspan_to_copy.size());
}
out_buf.reduce_size(cspan_to_copy.size());

return CHIP_NO_ERROR;
Expand All @@ -430,8 +438,13 @@ inline void CopyCharSpanToMutableCharSpanWithTruncation(CharSpan span_to_copy, M
{
size_t size_to_copy = std::min(span_to_copy.size(), out_span.size());

// There is no guarantee that span_to_copy and out_buf don't overlap, so use memmove()
memmove(out_span.data(), span_to_copy.data(), size_to_copy);
// No guarantee that span_to_copy and out_span don't overlap, so memmove() not memcpy().
// Either span may have a null data() when empty, which is undefined to pass to memmove() even at zero length.
// Check size_to_copy: it is zero when out_span is empty, even if span_to_copy is not.
if (size_to_copy != 0)
{
memmove(out_span.data(), span_to_copy.data(), size_to_copy);
}
out_span.reduce_size(size_to_copy);
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/support/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ chip_test_suite("tests") {
]

public_deps = [
":pw-test-macros",
"${chip_root}/src/app/common:cluster-objects",
"${chip_root}/src/credentials",
"${chip_root}/src/lib/core",
Expand Down
57 changes: 57 additions & 0 deletions src/lib/support/tests/TestSpan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include <lib/core/StringBuilderAdapters.h>
#include <lib/support/Span.h>
#include <lib/support/tests/ExtraPwTestMacros.h>

using namespace chip;

Expand Down Expand Up @@ -473,3 +474,59 @@ TEST(TestSpan, TestFromCharSpan)
// ByteSpan disallowed1 = ByteSpan::fromCharSpan(bytes);
// CharSpan disallowed2 = CharSpan::fromCharSpan<const uint8_t>(chars);
}

// A zero-length TLV string yields a { nullptr, 0 } span, so these cases drive that value through
// each copy helper. The assertions pin the normal contract; the null memmove argument itself is
// reported only by -fsanitize=undefined.
TEST(TestSpan, TestCopySpanToMutableSpanFromNullSource)
{
const ByteSpan nullSource;
ASSERT_EQ(nullSource.data(), nullptr);

uint8_t buf[4] = { 1, 2, 3, 4 };
MutableByteSpan out(buf);
EXPECT_SUCCESS(CopySpanToMutableSpan(nullSource, out));
EXPECT_TRUE(out.empty());
EXPECT_EQ(buf[0], static_cast<uint8_t>(1));

MutableByteSpan nullOut;
EXPECT_SUCCESS(CopySpanToMutableSpan(nullSource, nullOut));
EXPECT_TRUE(nullOut.empty());
}

TEST(TestSpan, TestCopyCharSpanToMutableCharSpanFromNullSource)
{
const CharSpan nullSource;
ASSERT_EQ(nullSource.data(), nullptr);

char buf[4] = { 'a', 'b', 'c', 'd' };
MutableCharSpan out(buf);
EXPECT_SUCCESS(CopyCharSpanToMutableCharSpan(nullSource, out));
EXPECT_TRUE(out.empty());
EXPECT_EQ(buf[0], 'a');

MutableCharSpan nullOut;
EXPECT_SUCCESS(CopyCharSpanToMutableCharSpan(nullSource, nullOut));
EXPECT_TRUE(nullOut.empty());
}

TEST(TestSpan, TestCopyCharSpanToMutableCharSpanWithTruncationFromNullSource)
{
const CharSpan nullSource;
ASSERT_EQ(nullSource.data(), nullptr);

char buf[4] = { 'a', 'b', 'c', 'd' };
MutableCharSpan out(buf);
CopyCharSpanToMutableCharSpanWithTruncation(nullSource, out);
EXPECT_TRUE(out.empty());
EXPECT_EQ(buf[0], 'a');

MutableCharSpan nullOut;
CopyCharSpanToMutableCharSpanWithTruncation(nullSource, nullOut);
EXPECT_TRUE(nullOut.empty());

// Truncation means a non-empty source can reach the copy with a zero-size destination.
MutableCharSpan nullOutFromNonEmpty;
CopyCharSpanToMutableCharSpanWithTruncation("abc"_span, nullOutFromNonEmpty);
EXPECT_TRUE(nullOutFromNonEmpty.empty());
}
Loading