diff --git a/lib/tdeck_ui/Telemetry/LocationShareState.cpp b/lib/tdeck_ui/Telemetry/LocationShareState.cpp index 2a4fefaf..24fa8dd0 100644 --- a/lib/tdeck_ui/Telemetry/LocationShareState.cpp +++ b/lib/tdeck_ui/Telemetry/LocationShareState.cpp @@ -15,7 +15,8 @@ bool effectiveTimestampMillis( const CustomLocationMeta& meta, uint64_t& timestamp_millis) { if (meta.has_timestamp) { - timestamp_millis = meta.timestamp_millis; + if (meta.timestamp_millis < 0) return false; + timestamp_millis = static_cast(meta.timestamp_millis); return true; } if (location.timestamp_seconds > @@ -55,7 +56,15 @@ std::size_t PeerLocationStore::firstVacant() const { return NO_SLOT; } -std::size_t PeerLocationStore::evictionCandidate() const { +std::size_t PeerLocationStore::evictionCandidate(uint64_t now_millis) const { + for (std::size_t index = 0; index < MAX_PEER_LOCATIONS; ++index) { + if (slots_[index].occupied && + slots_[index].record.has_expiry && + now_millis >= slots_[index].record.expires_at_millis) { + return index; + } + } + std::size_t candidate = NO_SLOT; for (std::size_t index = 0; index < MAX_PEER_LOCATIONS; ++index) { if (!slots_[index].occupied) continue; @@ -72,12 +81,12 @@ bool PeerLocationStore::visible( const PeerLocationRecord& record, uint64_t now_millis, uint64_t maximum_age_millis) { - if (record.expires_at_millis != 0 && + if (record.has_expiry && now_millis >= record.expires_at_millis) { return false; } - if (now_millis >= record.received_at_millis && - now_millis - record.received_at_millis > maximum_age_millis) { + if (now_millis >= record.source_timestamp_millis && + now_millis - record.source_timestamp_millis > maximum_age_millis) { return false; } return true; @@ -94,6 +103,11 @@ PeerLocationResult PeerLocationStore::apply( const LocationTelemetry& location, const CustomLocationMeta& meta, uint64_t received_at_millis) { + if ((meta.has_expires && meta.expires_millis < 0) || + (meta.has_approx_radius && meta.approx_radius_meters < 0)) { + return PeerLocationResult::INVALID_ARGUMENT; + } + uint64_t source_timestamp_millis = 0; if (!effectiveTimestampMillis(location, meta, source_timestamp_millis)) { return PeerLocationResult::INVALID_ARGUMENT; @@ -112,9 +126,10 @@ PeerLocationResult PeerLocationStore::apply( return PeerLocationResult::CEASED; } - const uint64_t expires_at_millis = - meta.has_expires ? meta.expires_millis : 0; - if (expires_at_millis != 0 && + const uint64_t expires_at_millis = meta.has_expires + ? static_cast(meta.expires_millis) + : 0; + if (meta.has_expires && received_at_millis >= expires_at_millis) { if (existing != NO_SLOT) clear(existing); return PeerLocationResult::EXPIRED; @@ -128,9 +143,12 @@ PeerLocationResult PeerLocationStore::apply( record.location = location; record.source_timestamp_millis = source_timestamp_millis; record.received_at_millis = received_at_millis; + record.has_expiry = meta.has_expires; record.expires_at_millis = expires_at_millis; record.approx_radius_meters = - meta.has_approx_radius ? meta.approx_radius_meters : 0; + meta.has_approx_radius + ? static_cast(meta.approx_radius_meters) + : 0; if (existing != NO_SLOT) { slots_[existing].record = record; @@ -138,7 +156,7 @@ PeerLocationResult PeerLocationStore::apply( } std::size_t target = firstVacant(); - if (target == NO_SLOT) target = evictionCandidate(); + if (target == NO_SLOT) target = evictionCandidate(received_at_millis); if (target == NO_SLOT) return PeerLocationResult::INVALID_ARGUMENT; if (!slots_[target].occupied) ++size_; slots_[target].record = record; diff --git a/lib/tdeck_ui/Telemetry/LocationShareState.h b/lib/tdeck_ui/Telemetry/LocationShareState.h index 41cc3891..a0be8999 100644 --- a/lib/tdeck_ui/Telemetry/LocationShareState.h +++ b/lib/tdeck_ui/Telemetry/LocationShareState.h @@ -21,6 +21,7 @@ struct PeerLocationRecord { LocationTelemetry location{}; uint64_t source_timestamp_millis = 0; uint64_t received_at_millis = 0; + bool has_expiry = false; uint64_t expires_at_millis = 0; uint32_t approx_radius_meters = 0; }; @@ -64,7 +65,7 @@ private: std::size_t find(const PeerId& peer) const; std::size_t firstVacant() const; - std::size_t evictionCandidate() const; + std::size_t evictionCandidate(uint64_t now_millis) const; static bool peerEquals(const PeerId& left, const PeerId& right); static bool visible(const PeerLocationRecord& record, uint64_t now_millis, diff --git a/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.cpp b/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.cpp index 3491959d..00a71a6f 100644 --- a/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.cpp +++ b/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.cpp @@ -589,24 +589,31 @@ CustomMetaResult decodeCustomLocationMeta( } candidate.has_cease = true; } else if (stringEquals(key, "expires", 7)) { - if (candidate.has_expires || - !cursor.readUnsigned(candidate.expires_millis)) { + uint64_t expires = 0; + if (candidate.has_expires || !cursor.readUnsigned(expires) || + expires > static_cast( + std::numeric_limits::max())) { return CustomMetaResult::MALFORMED; } + candidate.expires_millis = static_cast(expires); candidate.has_expires = true; } else if (stringEquals(key, "approxRadius", 12)) { uint64_t radius = 0; if (candidate.has_approx_radius || !cursor.readUnsigned(radius) || - radius > std::numeric_limits::max()) { + radius > static_cast( + std::numeric_limits::max())) { return CustomMetaResult::MALFORMED; } - candidate.approx_radius_meters = static_cast(radius); + candidate.approx_radius_meters = static_cast(radius); candidate.has_approx_radius = true; } else if (stringEquals(key, "ts", 2)) { - if (candidate.has_timestamp || - !cursor.readUnsigned(candidate.timestamp_millis)) { + uint64_t timestamp = 0; + if (candidate.has_timestamp || !cursor.readUnsigned(timestamp) || + timestamp > static_cast( + std::numeric_limits::max())) { return CustomMetaResult::MALFORMED; } + candidate.timestamp_millis = static_cast(timestamp); candidate.has_timestamp = true; } else if (!cursor.skipValue(0, skip_budget)) { return CustomMetaResult::MALFORMED; @@ -633,6 +640,11 @@ CustomMetaResult encodeCustomLocationMeta( return CustomMetaResult::EMPTY; } if (output == nullptr) return CustomMetaResult::INVALID_ARGUMENT; + if ((input.has_expires && input.expires_millis < 0) || + (input.has_approx_radius && input.approx_radius_meters < 0) || + (input.has_timestamp && input.timestamp_millis < 0)) { + return CustomMetaResult::INVALID_ARGUMENT; + } uint8_t temporary[MAX_ENCODED_CUSTOM_META]{}; Writer writer(temporary, sizeof(temporary)); @@ -643,15 +655,16 @@ CustomMetaResult encodeCustomLocationMeta( } if (input.has_expires) { ok = ok && writer.writeString("expires", 7) && - writer.writeUnsigned(input.expires_millis); + writer.writeUnsigned(static_cast(input.expires_millis)); } if (input.has_approx_radius) { ok = ok && writer.writeString("approxRadius", 12) && - writer.writeUnsigned(input.approx_radius_meters); + writer.writeUnsigned( + static_cast(input.approx_radius_meters)); } if (input.has_timestamp) { ok = ok && writer.writeString("ts", 2) && - writer.writeUnsigned(input.timestamp_millis); + writer.writeUnsigned(static_cast(input.timestamp_millis)); } if (!ok) return CustomMetaResult::INVALID_ARGUMENT; diff --git a/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.h b/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.h index d06570fb..bfa209e4 100644 --- a/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.h +++ b/lib/tdeck_ui/Telemetry/LocationTelemetryCodec.h @@ -28,11 +28,11 @@ struct CustomLocationMeta { bool has_cease = false; bool cease = false; bool has_expires = false; - uint64_t expires_millis = 0; + int64_t expires_millis = 0; bool has_approx_radius = false; - uint32_t approx_radius_meters = 0; + int32_t approx_radius_meters = 0; bool has_timestamp = false; - uint64_t timestamp_millis = 0; + int64_t timestamp_millis = 0; }; enum class DecodeResult : uint8_t { diff --git a/tests/native/test_custom_location_meta_codec.cpp b/tests/native/test_custom_location_meta_codec.cpp index 5c457fa7..b754b416 100644 --- a/tests/native/test_custom_location_meta_codec.cpp +++ b/tests/native/test_custom_location_meta_codec.cpp @@ -130,6 +130,15 @@ void rejectsMalformedAndPreservesOutput() { }; constexpr uint8_t too_many_entries[] = {0xde, 0x00, 0x11}; constexpr uint8_t huge_string[] = {0x81, 0xdb, 0xff, 0xff, 0xff, 0xff}; + constexpr uint8_t timestamp_above_columba_long[] = { + 0x81, 0xa2, 't', 's', + 0xcf, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + }; + constexpr uint8_t radius_above_columba_int[] = { + 0x81, + 0xac, 'a', 'p', 'p', 'r', 'o', 'x', 'R', 'a', 'd', 'i', 'u', 's', + 0xce, 0x80, 0x00, 0x00, 0x00, + }; struct Case { const uint8_t* data; std::size_t size; }; const Case cases[] = { @@ -139,6 +148,8 @@ void rejectsMalformedAndPreservesOutput() { {duplicate_cease, sizeof(duplicate_cease)}, {too_many_entries, sizeof(too_many_entries)}, {huge_string, sizeof(huge_string)}, + {timestamp_above_columba_long, sizeof(timestamp_above_columba_long)}, + {radius_above_columba_int, sizeof(radius_above_columba_int)}, }; const auto sentinel = expectedMeta(); for (const auto& item : cases) { @@ -165,6 +176,25 @@ void rejectsMalformedAndPreservesOutput() { CHECK(equalMeta(output, sentinel)); } +void rejectsOutboundValuesOutsideColumbaSignedDomains() { + uint8_t encoded[128]{}; + std::size_t written = 0; + + Telemetry::CustomLocationMeta meta{}; + meta.has_timestamp = true; + meta.timestamp_millis = -1; + CHECK(Telemetry::encodeCustomLocationMeta( + meta, encoded, sizeof(encoded), written) == + Telemetry::CustomMetaResult::INVALID_ARGUMENT); + + meta = Telemetry::CustomLocationMeta{}; + meta.has_approx_radius = true; + meta.approx_radius_meters = -1; + CHECK(Telemetry::encodeCustomLocationMeta( + meta, encoded, sizeof(encoded), written) == + Telemetry::CustomMetaResult::INVALID_ARGUMENT); +} + } // namespace int main() { @@ -172,6 +202,7 @@ int main() { distinguishesAbsentFalseTrueAndEmpty(); acceptsReorderedKeysAndSkipsUnknownNestedValues(); rejectsMalformedAndPreservesOutput(); + rejectsOutboundValuesOutsideColumbaSignedDomains(); std::cout << "custom location metadata codec: " << passed << " passed, " << failures << " failed\n"; return failures == 0 ? EXIT_SUCCESS : EXIT_FAILURE; diff --git a/tests/native/test_location_share_state.cpp b/tests/native/test_location_share_state.cpp index b1d9260e..541454ee 100644 --- a/tests/native/test_location_share_state.cpp +++ b/tests/native/test_location_share_state.cpp @@ -161,7 +161,7 @@ void enforcesExpiryAndStaleDisplayBoundaries() { CHECK(store.size() == 0); Telemetry::CustomLocationMeta no_meta{}; - CHECK(store.apply(id, location(20), no_meta, 1000) == + CHECK(store.apply(id, location(1), no_meta, 1000) == Telemetry::PeerLocationResult::INSERTED); CHECK(store.snapshot(1100, 100, snapshot, 2) == 1); CHECK(store.snapshot(1101, 100, snapshot, 2) == 0); @@ -170,6 +170,72 @@ void enforcesExpiryAndStaleDisplayBoundaries() { CHECK(store.prune(1101, 100) == 1); } +void basesFreshnessOnSenderCaptureTimeNotReceiptTime() { + Telemetry::PeerLocationStore store; + Telemetry::CustomLocationMeta no_meta{}; + const auto id = peer(31); + CHECK(store.apply(id, location(1), no_meta, 100000) == + Telemetry::PeerLocationResult::INSERTED); + + Telemetry::PeerLocationRecord snapshot[1]{}; + CHECK(store.snapshot(100000, 1000, snapshot, 1) == 0); + CHECK(store.prune(100000, 1000) == 1); + CHECK(!hasPeer(store, id)); +} + +void treatsPresentEpochZeroExpiryAsExpired() { + Telemetry::PeerLocationStore store; + auto meta = metaTimestamp(10000); + meta.has_expires = true; + meta.expires_millis = 0; + CHECK(store.apply(peer(32), location(10), meta, 1000) == + Telemetry::PeerLocationResult::EXPIRED); + CHECK(store.size() == 0); +} + +void reusesExpiredSlotsBeforeEvictingLiveRecords() { + Telemetry::PeerLocationStore store; + Telemetry::CustomLocationMeta no_meta{}; + for (std::size_t index = 0; index < Telemetry::MAX_PEER_LOCATIONS; ++index) { + Telemetry::CustomLocationMeta meta{}; + if (index + 1 == Telemetry::MAX_PEER_LOCATIONS) { + meta.has_expires = true; + meta.expires_millis = 1000; + } + CHECK(store.apply(peer(static_cast(index)), location(10), meta, + 100 + index) == + Telemetry::PeerLocationResult::INSERTED); + } + + CHECK(store.apply(peer(200), location(11), no_meta, 2000) == + Telemetry::PeerLocationResult::INSERTED); + CHECK(hasPeer(store, peer(0))); + CHECK(!hasPeer(store, peer(31))); + CHECK(hasPeer(store, peer(200))); + CHECK(store.size() == Telemetry::MAX_PEER_LOCATIONS); +} + +void rejectsDirectMetadataOutsideColumbaDomains() { + Telemetry::PeerLocationStore store; + auto meta = metaTimestamp(1); + meta.timestamp_millis = -1; + CHECK(store.apply(peer(33), location(1), meta, 1) == + Telemetry::PeerLocationResult::INVALID_ARGUMENT); + + meta = metaTimestamp(1); + meta.has_expires = true; + meta.expires_millis = -1; + CHECK(store.apply(peer(33), location(1), meta, 1) == + Telemetry::PeerLocationResult::INVALID_ARGUMENT); + + meta = metaTimestamp(1); + meta.has_approx_radius = true; + meta.approx_radius_meters = -1; + CHECK(store.apply(peer(33), location(1), meta, 1) == + Telemetry::PeerLocationResult::INVALID_ARGUMENT); + CHECK(store.size() == 0); +} + void expiredNewerUpdateClearsExistingState() { Telemetry::PeerLocationStore store; const auto id = peer(40); @@ -244,6 +310,10 @@ int main() { appliesOrderedCeaseWithoutTouchingOtherPeers(); reusesVacanciesBeforeDeterministicEviction(); enforcesExpiryAndStaleDisplayBoundaries(); + basesFreshnessOnSenderCaptureTimeNotReceiptTime(); + treatsPresentEpochZeroExpiryAsExpired(); + reusesExpiredSlotsBeforeEvictingLiveRecords(); + rejectsDirectMetadataOutsideColumbaDomains(); expiredNewerUpdateClearsExistingState(); snapshotsAreCallerOwnedAndCapacityBounded(); rejectsTimestampOverflowWithoutMutation();