libp2p / libp2p/cpp-libp2p

Signed overflow in InmemAddressRepository::upsertAddresses

Open
#281 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug good first issue
Dominant language
C++
Stars
492
Forks
130
PR merge metrics
No merged PRs in 30d

Description

Summary

The following line
https://github.com/libp2p/cpp-libp2p/blob/093a3f7b0c9f45300f2ec88cc2bfabe083c5d2f2/src/peer/address_repository/inmem_address_repository.cpp#L103
Provokes a signed overflow, which is reported by UBSAN, if TTL is larger than several hundred years. In particular, libp2p::peer::ttl::kPermanent does trigger the overflow.
https://godbolt.org/z/ahcdE5Msq - an MRE.
Permanent ttl should be expressed in some valid way, and probably some type safer than chrono::milliseconds, that silently overflows, should be used in the interface.

Expected behavior

Expected defined behaviour.

Actual behavior

Actual behaviour is undefined due to signed overflow.

Relevant log output
/usr/lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/chrono.h:227:38: runtime error: signed integer overflow: 9223372036854775807 * 1000000 cannot be represented in type 'long'
24.11.28 14:52:30.896390  telemetry        Info      TelemetryConnection#1  Connection established
    #0 0x61348f7c8555 in std::chrono::duration<long, std::ratio<1l, 1000000000l>> std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, std::ratio<1000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1000l>>(std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /usr/lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/chrono.h:227:38
    #1 0x61348f7c8250 in std::enable_if<__is_duration<std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::value, std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::type std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, long, std::ratio<1l, 1000l>>(std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /usr/lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/chrono.h:289:11
    #2 0x61348fa7e150 in std::chrono::duration<long, std::ratio<1l, 1000000000l>>::duration<long, std::ratio<1l, 1000l>, void>(std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /usr/lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/chrono.h:582:10
    #3 0x61348fea5010 in std::common_type<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, std::chrono::duration<long, std::ratio<1l, 1000l>>>::type std::chrono::operator+<long, std::ratio<1l, 1000000000l>, long, std::ratio<1l, 1000l>>(std::chrono::duration<long, std::ratio<1l, 1000000000l>> const&, std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /usr/lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/chrono.h:702:36
    #4 0x61348fea215b in std::chrono::time_point<std::chrono::_V2::steady_clock, std::common_type<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, std::chrono::duration<long, std::ratio<1l, 1000l>>>::type> std::chrono::operator+<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l>>, long, std::ratio<1l, 1000l>>(std::chrono::time_point<std::chrono::_V2::steady_clock, std::chrono::duration<long, std::ratio<1l, 1000000000l>>> const&, std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /usr/lib64/gcc/x86_64-pc-linux-gnu/14.2.1/../../../../include/c++/14.2.1/bits/chrono.h:1107:47
    #5 0x6134996c8f07 in libp2p::peer::InmemAddressRepository::upsertAddresses(libp2p::peer::PeerId const&, std::span<libp2p::multi::Multiaddress const, 18446744073709551615ul>, std::chrono::duration<long, std::ratio<1l, 1000l>>) /home/harrm/.hunter/_Base/3fb5849/417772a/9a920dc/Build/libp2p/Source/src/peer/address_repository/inmem_address_repository.cpp:103:36
...
Possible Solution

No response

Version

Commit c96d45f792fafd6970a7e37ec816b02a9167e2b6

Would you like to work on fixing this bug ?

Yes

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/peer/address_repository/inmem_address_repository.cpp at the linked line and inspect how TTL is added to the steady-clock time point. Use the provided Godbolt MRE and UBSAN report to reproduce the overflow; done means permanent and very large TTL values have defined behavior without signed overflow, with any interface changes accounted for.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.