Skip to content

[CHIPTOOL] Fix AddArgument for ByteSpan when content is hexstring #38838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[CHIPTOOL] Fix AddArgument for ByteSpan when content is hexstring #38838

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 31 additions & 0 deletions examples/chip-tool/commands/common/Command.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,37 @@ size_t Command::AddArgument(const char * name, chip::CharSpan * value, const cha
size_t Command::AddArgument(const char * name, chip::ByteSpan * value, const char * desc, uint8_t flags)
{
Argument arg;

// if the value is a hex string, we need to convert it from hex to bytes
if ((value->size() > kHexStringPrefixLen) && IsHexString((const char *) value->data()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes no sense. The way this works is that AddArgument registers a memory location where the value will live later when we have a value. At the point where this is called, value->size() is 0 at best (if it's a non-optional argument), and garbage at worst, because value might in fact be a reinterpret_cast pointer to an Optional<ByteSpan> here (if the kOptional flag is set).

The hex string detection should (and does) happen during the actual argument decoding, which is what will fill in the ByteSpan here. In particular, this will happen in Command::InitArgument.

{
uint8_t * buffer = nullptr;
size_t size;

CHIP_ERROR err = HexToBytes(
chip::CharSpan((const char *) (&value->data()[kHexStringPrefixLen]), (value->size() - kHexStringPrefixLen)),
[&buffer](size_t allocSize) {
buffer = static_cast<uint8_t *>(chip::Platform::MemoryCalloc(allocSize, sizeof(uint8_t)));
return buffer;
},
&size);

if (err != CHIP_NO_ERROR)
{
if (buffer != nullptr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's log the error.

{
chip::Platform::MemoryFree(buffer);
}

return 0;
}

// The buffer is now filled with the decoded bytes, so we can set the value
// to point to the buffer and set the size accordingly
chip::ByteSpan newvalue = new chip::ByteSpan(buffer, size);
value = &newvalue;
}

arg.type = ArgumentType::OctetString;
arg.name = name;
arg.value = reinterpret_cast<void *>(value);
Expand Down
Loading