HomePhabricator

Fix a potential segfault in the seeder

Description

Fix a potential segfault in the seeder

Summary:
The seeded software is doing statistics on tested nodes before any node get tested.
This diff addresses the issue by ensuring that the list of tested node is not empty before statistics are built on top.

Implementation details:
This bug has been revealed by running the seeder on OSX.
The CAddrDb::GetStats() method can access internal dequeue and map
members by index despite their size is zero, causing a segfault.

This diff fixes it by:

  • Testing the size of the list of tested nodes to be > 0 before accessing, ensuring data exists before building the stats.
  • Using at() for accessing instead of []. This function checks for out-of-bounds accesses and will throw an exception instead of a segfault, making the debugging much easier. It also avoids adding inadvertently an entry in the map (this is also enforced by making the method const).

Test Plan:
Build with various compilers and run the seeder on various platforms.
Currently tested on Linux and OSX (the seeder is unsupported on
Windows).
Note: not all combinations would give a segfault before this patch.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2727

Details

Provenance
FabienAuthored on Mar 22 2019, 12:00
FabienPushed on Mar 28 2019, 11:33
Reviewer
Restricted Project
Differential Revision
D2727: Fix a potential segfault in the seeder
Parents
rABCb9c102a733b9: Remove nonnull warning when calling secp256k1_schnorr_sign with NULL noncefp
Branches
Unknown
Tags
Unknown