added a test case for it and updated the doc.
rbtree/domaintree was modified to avoid such operations.
isc::dns::LabelSequence target_labels(target_name);
int order = -1;
+ // For possible LabelSequence serialization we always store labels data
+ // in the separate local buffer.
+ uint8_t labels_buf[dns::LabelSequence::MAX_SERIALIZED_LENGTH];
while (current != NULL) {
- const dns::LabelSequence current_labels(current->getLabels());
+ const dns::LabelSequence current_labels(
+ dns::LabelSequence(current->getLabels(), labels_buf));
const isc::dns::NameComparisonResult compare_result =
target_labels.compare(current_labels);
const isc::dns::NameComparisonResult::NameRelation relation =
isc::dns::LabelSequence target_labels(target_name);
int order = -1;
+ // For possible LabelSequence serialization we always store labels data
+ // in the separate local buffer.
+ uint8_t labels_buf[dns::LabelSequence::MAX_SERIALIZED_LENGTH];
while (current != NULL) {
- const dns::LabelSequence current_labels(current->getLabels());
+ const dns::LabelSequence current_labels(
+ dns::LabelSequence(current->getLabels(), labels_buf));
const isc::dns::NameComparisonResult compare_result =
target_labels.compare(current_labels);
const isc::dns::NameComparisonResult::NameRelation relation =
return (1 + getLabelCount() + getDataLength());
}
+namespace {
+// Check if buf is not in the range of [bp, ep), which means
+// - end of buffer is before bp, or
+// - beginning of buffer is on or after ep
+bool
+isOutOfRange(const uint8_t* bp, const uint8_t* ep,
+ const uint8_t* buf, size_t buf_len)
+{
+ return (bp >= buf + buf_len || // end of buffer is before bp
+ ep <= buf); // beginning of buffer is on or after ep
+}
+}
+
void
LabelSequence::serialize(void* buf, size_t buf_len) const {
const size_t expected_size = getSerializedLength();
const size_t offsets_len = getLabelCount();
assert(offsets_len < 256); // should be in the 8-bit range
+ // Overridden check. Buffer shouldn't overwrap the offset of name data
+ // regions.
uint8_t* bp = reinterpret_cast<uint8_t*>(buf);
+ const size_t ndata_len = getDataLength();
+ if (!isOutOfRange(offsets_, offsets_ + offsets_len, bp, buf_len) ||
+ !isOutOfRange(data_, data_ + ndata_len, bp, buf_len)) {
+ isc_throw(BadValue, "serialize would break the source sequence");
+ }
+
*bp++ = offsets_len;
for (size_t i = 0; i < offsets_len; ++i) {
*bp++ = offsets_[first_label_ + i] - offsets_[first_label_];
}
- const size_t ndata_len = getDataLength();
std::memcpy(bp, &data_[offsets_[first_label_]], ndata_len);
bp += ndata_len;
/// the value returned by getSerializedLength() (it can be larger than
/// that).
///
+ /// Be careful about where the buffer is located; due to the nature
+ /// of the buffer, it's quite possible that the memory region is being used
+ /// to construct another active \c LabelSequence. In such a case
+ /// the serialization would silently break that sequence object, and
+ /// it will be very difficult to identify the cause. This method
+ /// has minimal level checks to avoid such disruption: If the serialization
+ /// would break "this" \c LabelSequence object, it doesn't write anything
+ /// to the given buffer and throw a \c isc::BadValue exception.
+ ///
+ /// In general, it should be safe to call this method on a
+ /// \c LabelSequence object constructed from a \c Name object or
+ /// a copy of such \c LabelSequence. When you construct \c LabelSequence
+ /// from pre-serialized data, calling this method on it can be unsafe.
+ /// One safe (but a bit less efficient) way in such a case is to make
+ /// the source \c LabelSequence temporary and immediately create a
+ /// local copy using an explicit buffer, and call this method on the
+ /// latter:
+ /// \code
+ /// // don't do this, it's not safe (and would result in exception):
+ /// // LabelSequence(buf).serialize(buf, buf_len);
+ ///
+ /// // The following are the safe way:
+ /// uint8_t ext_buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+ /// LabelSequence seq(LabelSequence(buf), ext_buf);
+ /// ... (strip the labels, etc)
+ /// seq.serialize(buf, buf_len); // it's safe to override buf here
+ /// \endcode
+ ///
/// The serialized image would be as follows:
/// - olen: number of offsets (1 byte)
/// - binary sequence of offsets (olen bytes, verbatim copy of offsets_
/// - binary sequence of name data (length determined by itself, verbatim
/// copy of data_ of the corresponding size)
///
- /// Applications must use the resulting image opaque value and must not
+ /// Applications must use the resulting image as opaque value and must not
/// use it for other purposes than input to the corresponding constructor
/// to restore it. Application behavior that assumes the specific
/// organization of the image is not guaranteed.
///
/// \throw isc::BadValue buf_len is too short (this method never throws
- /// otherwise)
+ /// otherwise) or the serialization would override internal data of
+ /// of the source LabelSequence.
///
/// \param buf Pointer to the placeholder to dump the serialized image
/// \param buf_len The size of available region in \c buf
}
TEST_F(LabelSequenceTest, serialize) {
- // placeholder for serialized data
- uint8_t labels_buf[LabelSequence::MAX_SERIALIZED_LENGTH];
+ // placeholder for serialized data. We use a sufficiently large space
+ // for testing the overwrapping cases below.
+ uint8_t labels_buf[LabelSequence::MAX_SERIALIZED_LENGTH * 3];
// vector to store expected and actual data
vector<LabelSequence> actual_labelseqs;
EXPECT_EQ(NameComparisonResult::EQUAL,
LabelSequence(labels_buf).compare(*itl).getRelation());
+
+ // Shift the data to the middle of the buffer for overwrap check
+ uint8_t* const bp = labels_buf;
+ std::memcpy(bp + serialized_len, bp, serialized_len);
+ // Memory layout is now as follows:
+ // <- ser_len -> <- ser_len ------>
+ // bp bp+ser_len bp+(ser_len*2)
+ // olen,odata,ndata
+
+ // end of buffer would be the first byte of offsets: invalid.
+ EXPECT_THROW(LabelSequence(bp + serialized_len).
+ serialize(bp + 2, serialized_len),
+ isc::BadValue);
+ // begin of buffer would be the last byte of ndata: invalid.
+ EXPECT_THROW(LabelSequence(bp + serialized_len).
+ serialize(bp + (2 * serialized_len) - 1, serialized_len),
+ isc::BadValue);
+ // A boundary safe case: buffer is placed after the sequence data.
+ // should cause no disruption.
+ LabelSequence(bp + serialized_len).
+ serialize(bp + 2 * serialized_len, serialized_len);
+ // A boundary safe case: buffer is placed before the sequence data
+ // should cause no disruption. (but the original serialized data will
+ // be overridden, so it can't be used any more)
+ LabelSequence(bp + serialized_len).
+ serialize(bp + 1, serialized_len);
}
EXPECT_THROW(ls1.serialize(labels_buf, ls1.getSerializedLength() - 1),
projects / thirdparty / kea.git / commitdiff
