fuzz-tests: Improve the fuzz-bolt12-invrequest-decode wire test
#8404
Conversation
|
Doesn't fuzz-bolt12-invrequest-decode.c already cover most of what is being tested here, plus more in |
Right, I guess it makes more sense then to replicate this test as an improvement to |
Chand-ra
force-pushed
the
tlv_invoice_request
branch
from
1a8f7a6 to
48bf887
Compare
Chand-ra
changed the title
struct tlv_invoice_requestfuzz-bolt12-invrequest-decode wire test
| static bool sciddir_or_pubkey_eq(const struct sciddir_or_pubkey *a, | ||
| const struct sciddir_or_pubkey *b) | ||
| { | ||
| return memcmp(a, b, sizeof(*a)) == 0; |
There was a problem hiding this comment.
Are we sure the full sciddir_or_pubkey struct gets initialized? It looks like only one of sciddir or pubkey is expected to be present, so maybe we could end up comparing uninitialized memory here...
| MEM_EQ(offer_issuer); | ||
| VAL_EQ(offer_quantity_max); | ||
| PTR_EQ(offer_issuer_id, pubkey_eq); | ||
| STRUCT_EQ(offer_recurrence, struct recurrence); |
There was a problem hiding this comment.
It looks like struct recurrence contains 5 bytes of data, which means it probably gets padded to 8 bytes. We can't safely memcmp in this case, as there's 3 bytes of uninitialized memory.
| VAL_EQ(offer_quantity_max); | ||
| PTR_EQ(offer_issuer_id, pubkey_eq); | ||
| STRUCT_EQ(offer_recurrence, struct recurrence); | ||
| STRUCT_EQ(offer_recurrence_paywindow, struct recurrence_paywindow); |
There was a problem hiding this comment.
Likewise, struct recurrence paywindow is 9 bytes and likely unsafe to memcmp.
| STRUCT_EQ(offer_recurrence, struct recurrence); | ||
| STRUCT_EQ(offer_recurrence_paywindow, struct recurrence_paywindow); | ||
| VAL_EQ(offer_recurrence_limit); | ||
| STRUCT_EQ(offer_recurrence_base, struct recurrence_base); |
There was a problem hiding this comment.
struct offer_recurrence_base is 9 bytes and likely unsafe to memcmp.
Changelog-None: Currently, the `BOLT ElementsProject#12` invrequest parsing test only tests the invrequest decode function. Add a test for the encoding function as well by making the test roundtrip.
Chand-ra
force-pushed
the
tlv_invoice_request
branch
from
48bf887 to
384ffd8
Compare
There was a problem hiding this comment.
The target LGTM.
Within a couple minutes of fuzzing, I was able to find inputs with more coverage:
#1485 INITED cov: 607 ft: 2022 corp: 251/22Kb exec/s: 0 rss: 179Mb
...
#424149 RELOAD cov: 620 ft: 2105 corp: 286/26Kb lim: 4096 exec/s: 2754 rss: 632Mb
Could you run the target a little longer and re-merge the new inputs?
|
After an hour I got even more coverage: |
Improvements in the fuzz-testing scheme of `fuzz-bolt12-invrequest-decode` led to the discovery of test inputs that result in greater in code coverage. Add these inputs to the test's seed corpus.
Chand-ra
force-pushed
the
tlv_invoice_request
branch
from
384ffd8 to
86d9fa8
Compare
Currently, the
BOLT #12invrequest parsing test only tests the invrequest decode function. Add a test for the encoding function as well by making the test roundtrip.Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
CC: @morehouse