Skip to content

Commit

Permalink
Add a buffer reader helper class that can be used to read buffers saf…
Browse files Browse the repository at this point in the history
…ely.

The idea is to not have people doing ad-hoc, and often incorrect when
they forget to update lengths, length checks.

I did measure the codesize with the Read methods out-of-line, and it's
larger than with the inline methods as far as I can tell.  It might be
worth re-measuring once we have more callers to see what things look
like then.
  • Loading branch information
bzbarsky-apple committed Oct 27, 2020
1 parent 84994f2 commit f63ccce
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 29 deletions.
1 change: 1 addition & 0 deletions src/lib/support/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ static_library("support") {
"Base64.h",
"BitFlags.h",
"BufBound.h",
"BufferReader.h",
"CHIPArgParser.cpp",
"CHIPCounter.cpp",
"CHIPCounter.h",
Expand Down
150 changes: 150 additions & 0 deletions src/lib/support/BufferReader.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @file
* Utility classes for safely reading from size-limited buffers.
*/

#pragma once

#include <climits>
#include <lib/core/CHIPEncoding.h>
#include <lib/core/CHIPError.h>
#include <lib/support/CodeUtils.h>
#include <stdint.h>

namespace chip {
namespace Encoding {
namespace LittleEndian {

/**
* @class Reader
*
* Simple reader for reading little-endian things out of buffers.
*/
class Reader
{
public:
/**
* Create a data model reader from a given buffer and length.
*
* @param buffer The octet buffer to read from. The caller must ensure
* (most simply by allocating the reader on the stack) that
* the buffer outlives the reader. The buffer is allowed to
* be null if buf_len is 0.
* @param buf_len The number of octets in the buffer.
*/
Reader(const uint8_t * buffer, uint16_t buf_len) : mBufStart(buffer), mReadPtr(buffer), mAvailable(buf_len) {}

/**
* Number of octets we have read so far. This might be able to go away once
* we do less switching back and forth between DataModelReader and raw
* buffers.
*/
uint16_t OctetsRead() const { return static_cast<uint16_t>(mReadPtr - mBufStart); }

/**
* Read a single 8-bit unsigned integer.
*
* @param [out] dest Where the 8-bit integer goes.
*
* @return Whether the read succeeded. The read can fail if there are not
* enough octets available.
*/
CHECK_RETURN_VALUE
CHIP_ERROR Read8(uint8_t * dest) { return Read(dest); }

/**
* Read a single 16-bit unsigned integer.
*
* @param [out] dest Where the 16-bit integer goes.
*
* @return Whether the read succeeded. The read can fail if there are not
* enough octets available.
*/
CHECK_RETURN_VALUE
CHIP_ERROR Read16(uint16_t * dest) { return Read(dest); }

/**
* Read a single 32-bit unsigned integer.
*
* @param [out] dest Where the 32-bit integer goes.
*
* @return Whether the read succeeded. The read can fail if there are not
* enough octets available.
*/
CHECK_RETURN_VALUE
CHIP_ERROR Read32(uint32_t * dest) { return Read(dest); }

/**
* Read a single 64-bit unsigned integer.
*
* @param [out] dest Where the 64-bit integer goes.
*
* @return Whether the read succeeded. The read can fail if there are not
* enough octets available.
*/
CHECK_RETURN_VALUE
CHIP_ERROR Read64(uint64_t * dest) { return Read(dest); }

protected:
template <typename T>
CHIP_ERROR Read(T * retval)
{
static_assert(CHAR_BIT == 8, "Our various sizeof checks rely on bytes and octets being the same thing");

static constexpr size_t data_size = sizeof(T);

if (mAvailable < data_size)
{
return CHIP_ERROR_BUFFER_TOO_SMALL;
}

Read(mReadPtr, retval);
mAvailable = static_cast<uint16_t>(mAvailable - data_size);
return CHIP_NO_ERROR;
}

private:
// These helper methods return void and put the value being read into an
// outparam because that allows us to easily overload on the type of the
// thing being read.
void Read(const uint8_t *& p, uint8_t * dest) { *dest = Encoding::Read8(p); }
void Read(const uint8_t *& p, uint16_t * dest) { *dest = Encoding::LittleEndian::Read16(p); }
void Read(const uint8_t *& p, uint32_t * dest) { *dest = Encoding::LittleEndian::Read32(p); }
void Read(const uint8_t *& p, uint64_t * dest) { *dest = Encoding::LittleEndian::Read64(p); }

/**
* Our buffer start.
*/
const uint8_t * const mBufStart;

/**
* Our current read point.
*/
const uint8_t * mReadPtr;

/**
* The number of octets we can still read starting at mReadPtr.
*/
uint16_t mAvailable;
};

} // namespace LittleEndian
} // namespace Encoding
} // namespace chip
72 changes: 43 additions & 29 deletions src/transport/raw/MessageHeader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@

#include <type_traits>

#include <core/CHIPEncoding.h>
#include <core/CHIPError.h>
#include <support/BufferReader.h>
#include <support/CodeUtils.h>

/**********************************************
Expand Down Expand Up @@ -137,27 +137,29 @@ uint16_t MessageAuthenticationCode::TagLenForEncryptionType(Header::EncryptionTy

CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint16_t * decode_len)
{
CHIP_ERROR err = CHIP_NO_ERROR;
const uint8_t * p = data;
CHIP_ERROR err = CHIP_NO_ERROR;
LittleEndian::Reader reader(data, size);
int version;

VerifyOrExit(size >= kFixedUnencryptedHeaderSizeBytes, err = CHIP_ERROR_INVALID_ARGUMENT);
uint16_t octets_read;

uint16_t header;
header = LittleEndian::Read16(p);
err = reader.Read16(&header);
SuccessOrExit(err);
version = ((header & kVersionMask) >> kVersionShift);
VerifyOrExit(version == kHeaderVersion, err = CHIP_ERROR_VERSION_MISMATCH);

mEncryptionType = static_cast<Header::EncryptionType>((header & kEncryptionTypeMask) >> kEncryptionTypeShift);
mFlags.SetRaw(header & Header::kFlagsMask);

mMessageId = LittleEndian::Read32(p);
err = reader.Read32(&mMessageId);
SuccessOrExit(err);

if (mFlags.Has(Header::FlagValues::kSourceNodeIdPresent))
{
static_assert(kNodeIdSizeBytes == 8, "Read64 might read more bytes than we have in the buffer");
VerifyOrExit(size >= static_cast<size_t>(p - data) + kNodeIdSizeBytes, err = CHIP_ERROR_INVALID_ARGUMENT);
mSourceNodeId.SetValue(LittleEndian::Read64(p));
uint64_t sourceNodeId;
err = reader.Read64(&sourceNodeId);
SuccessOrExit(err);
mSourceNodeId.SetValue(sourceNodeId);
}
else
{
Expand All @@ -166,20 +168,25 @@ CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint1

if (mFlags.Has(Header::FlagValues::kDestinationNodeIdPresent))
{
VerifyOrExit(size >= static_cast<size_t>(p - data) + kNodeIdSizeBytes, err = CHIP_ERROR_INVALID_ARGUMENT);
mDestinationNodeId.SetValue(LittleEndian::Read64(p));
uint64_t destinationNodeId;
err = reader.Read64(&destinationNodeId);
SuccessOrExit(err);
mDestinationNodeId.SetValue(destinationNodeId);
}
else
{
mDestinationNodeId.ClearValue();
}

VerifyOrExit(size >= static_cast<size_t>(p - data) + sizeof(uint16_t) * 2, err = CHIP_ERROR_INVALID_ARGUMENT);
mEncryptionKeyID = LittleEndian::Read16(p);
mPayloadLength = LittleEndian::Read16(p);
err = reader.Read16(&mEncryptionKeyID);
SuccessOrExit(err);

VerifyOrExit(p - data == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL);
*decode_len = static_cast<uint16_t>(p - data);
err = reader.Read16(&mPayloadLength);
SuccessOrExit(err);

octets_read = reader.OctetsRead();
VerifyOrExit(octets_read == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL);
*decode_len = octets_read;

exit:

Expand All @@ -188,20 +195,26 @@ CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint1

CHIP_ERROR PayloadHeader::Decode(Header::Flags flags, const uint8_t * const data, uint16_t size, uint16_t * decode_len)
{
CHIP_ERROR err = CHIP_NO_ERROR;
const uint8_t * p = data;
CHIP_ERROR err = CHIP_NO_ERROR;
LittleEndian::Reader reader(data, size);
uint8_t header;
uint16_t octets_read;

err = reader.Read8(&header);
SuccessOrExit(err);

VerifyOrExit(size >= kEncryptedHeaderSizeBytes, err = CHIP_ERROR_INVALID_ARGUMENT);
err = reader.Read8(&mMessageType);
SuccessOrExit(err);

header = Read8(p);
mMessageType = Read8(p);
mExchangeID = LittleEndian::Read16(p);
err = reader.Read16(&mExchangeID);
SuccessOrExit(err);

if (flags.Has(Header::FlagValues::kVendorIdPresent))
{
VerifyOrExit(size >= static_cast<size_t>(p - data) + kVendorIdSizeBytes, err = CHIP_ERROR_INVALID_ARGUMENT);
mVendorId.SetValue(LittleEndian::Read16(p));
uint16_t vendor_id;
err = reader.Read16(&vendor_id);
SuccessOrExit(err);
mVendorId.SetValue(vendor_id);
}
else
{
Expand All @@ -210,11 +223,12 @@ CHIP_ERROR PayloadHeader::Decode(Header::Flags flags, const uint8_t * const data

mExchangeFlags.SetRaw(header);

VerifyOrExit(size >= static_cast<size_t>(p - data) + sizeof(uint16_t), err = CHIP_ERROR_INVALID_ARGUMENT);
mProtocolID = LittleEndian::Read16(p);
err = reader.Read16(&mProtocolID);
SuccessOrExit(err);

VerifyOrExit(p - data == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL);
*decode_len = static_cast<uint16_t>(p - data);
octets_read = reader.OctetsRead();
VerifyOrExit(octets_read == EncodeSizeBytes(), err = CHIP_ERROR_INTERNAL);
*decode_len = octets_read;

exit:

Expand Down

0 comments on commit f63ccce

Please sign in to comment.