[fido] CHECK that Append ranges don't overlap

This change introduces a CHECK that the ranges passed to
u2f_parsing_utils::Append do not overlap. Calling it with overlapping
ranges results in undefined behavior if a reallocation is necessary
during the Append operation.

Bug: 780078
Change-Id: Ibc83c054bdf8dee81f293c54f980c6735fcc358a
Reviewed-on: https://chromium-review.googlesource.com/999481
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548743}

device/fido/u2f_parsing_utils.cc

@@ -9,6 +9,16 @@
 namespace device {
 namespace u2f_parsing_utils {
 
+namespace {
+
+constexpr bool AreSpansDisjoint(base::span<const uint8_t> lhs,
+                                base::span<const uint8_t> rhs) {
+  return lhs.data() + lhs.size() <= rhs.data() ||  // [lhs)...[rhs)
+         rhs.data() + rhs.size() <= lhs.data();    // [rhs)...[lhs)
+}
+
+}  // namespace
+
 const uint32_t kU2fResponseKeyHandleLengthPos = 66u;
 const uint32_t kU2fResponseKeyHandleStartPos = 67u;
 const char kEs256[] = "ES256";
@@ -25,6 +35,7 @@ base::Optional<std::vector<uint8_t>> MaterializeOrNull(
 }
 
 void Append(std::vector<uint8_t>* target, base::span<const uint8_t> in_values) {
+  CHECK(AreSpansDisjoint(*target, in_values));
   target->insert(target->end(), in_values.begin(), in_values.end());
 }
 

device/fido/u2f_parsing_utils_unittest.cc

@@ -53,6 +53,17 @@ TEST(U2fParsingUtils, Append) {
   EXPECT_THAT(target, ::testing::ElementsAreArray(kOneTwoThree));
 }
 
+TEST(U2fParsingUtils, AppendSelfCrashes) {
+  std::vector<uint8_t> target(std::begin(kOneTwoThree), std::end(kOneTwoThree));
+  auto span = base::make_span(target);
+
+  // Tests the case where |in_values| overlap with the beginning of |*target|.
+  EXPECT_DEATH_IF_SUPPORTED(Append(&target, span.first(1)), "Check failed");
+
+  // Tests the case where |in_values| overlap with the end of |*target|.
+  EXPECT_DEATH_IF_SUPPORTED(Append(&target, span.last(1)), "Check failed");
+}
+
 // ExtractSpan and ExtractSuffixSpan are implicitly tested as they used by
 // the Extract and ExtractSuffix implementations.