Skip to content

Commit

Permalink
Ensure that attempts to read integers from TLV don't silently produce…
Browse files Browse the repository at this point in the history
… unexpected values. (#9944)

Two changes:

1) Ensure the value was encoded with the signed-ness the reader expects.

2) Ensure the value is in the range the reader expects instad of
   ending up with overflow and whatever that does..

Fixes #5152

The test changes were largely lifted from
#5764
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 28, 2021
1 parent 8e9a62b commit 903632d
Show file tree
Hide file tree
Showing 2 changed files with 301 additions and 32 deletions.
77 changes: 54 additions & 23 deletions src/lib/core/CHIPTLVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,74 +130,105 @@ CHIP_ERROR TLVReader::Get(bool & v)

CHIP_ERROR TLVReader::Get(int8_t & v)
{
uint64_t v64 = 0;
int64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = CastToSigned(static_cast<uint8_t>(v64));
if (!CanCastTo<int8_t>(v64))
{
return CHIP_ERROR_INVALID_INTEGER_VALUE;
}
v = static_cast<int8_t>(v64);
return err;
}

CHIP_ERROR TLVReader::Get(int16_t & v)
{
uint64_t v64 = 0;
int64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = CastToSigned(static_cast<uint16_t>(v64));
if (!CanCastTo<int16_t>(v64))
{
return CHIP_ERROR_INVALID_INTEGER_VALUE;
}
v = static_cast<int16_t>(v64);
return err;
}

CHIP_ERROR TLVReader::Get(int32_t & v)
{
uint64_t v64 = 0;
int64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = CastToSigned(static_cast<uint32_t>(v64));
if (!CanCastTo<int32_t>(v64))
{
return CHIP_ERROR_INVALID_INTEGER_VALUE;
}
v = static_cast<int32_t>(v64);
return err;
}

CHIP_ERROR TLVReader::Get(int64_t & v)
{
uint64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = CastToSigned(v64);
return err;
// Internal callers of this method depend on it not modifying "v" on failure.
switch (ElementType())
{
case TLVElementType::Int8:
v = CastToSigned(static_cast<uint8_t>(mElemLenOrVal));
break;
case TLVElementType::Int16:
v = CastToSigned(static_cast<uint16_t>(mElemLenOrVal));
break;
case TLVElementType::Int32:
v = CastToSigned(static_cast<uint32_t>(mElemLenOrVal));
break;
case TLVElementType::Int64:
v = CastToSigned(mElemLenOrVal);
break;
default:
return CHIP_ERROR_WRONG_TLV_TYPE;
}

return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Get(uint8_t & v)
{
uint64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = static_cast<uint8_t>(v64);
if (!CanCastTo<uint8_t>(v64))
{
return CHIP_ERROR_INVALID_INTEGER_VALUE;
}
v = static_cast<uint8_t>(v64);
return err;
}

CHIP_ERROR TLVReader::Get(uint16_t & v)
{
uint64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = static_cast<uint16_t>(v64);
if (!CanCastTo<uint16_t>(v64))
{
return CHIP_ERROR_INVALID_INTEGER_VALUE;
}
v = static_cast<uint16_t>(v64);
return err;
}

CHIP_ERROR TLVReader::Get(uint32_t & v)
{
uint64_t v64 = 0;
CHIP_ERROR err = Get(v64);
v = static_cast<uint32_t>(v64);
if (!CanCastTo<uint32_t>(v64))
{
return CHIP_ERROR_INVALID_INTEGER_VALUE;
}
v = static_cast<uint32_t>(v64);
return err;
}

CHIP_ERROR TLVReader::Get(uint64_t & v)
{
// Internal callers of this method depend on it not modifying "v" on failure.
switch (ElementType())
{
case TLVElementType::Int8:
v = static_cast<uint64_t>(static_cast<int64_t>(CastToSigned(static_cast<uint8_t>(mElemLenOrVal))));
break;
case TLVElementType::Int16:
v = static_cast<uint64_t>(static_cast<int64_t>(CastToSigned(static_cast<uint16_t>(mElemLenOrVal))));
break;
case TLVElementType::Int32:
v = static_cast<uint64_t>(static_cast<int64_t>(CastToSigned(static_cast<uint32_t>(mElemLenOrVal))));
break;
case TLVElementType::Int64:
case TLVElementType::UInt8:
case TLVElementType::UInt16:
case TLVElementType::UInt32:
Expand Down
Loading

0 comments on commit 903632d

Please sign in to comment.