--- Day changed Wed Apr 15 2020 00:00 -!- jonatack [~jon@37.170.227.51] has joined #bitcoin-core-pr-reviews 00:11 -!- lightlike_ [~lightlike@p200300C7EF10B500CCE41A8C08B0A787.dip0.t-ipconnect.de] has quit [Ping timeout: 256 seconds] 00:29 -!- jonatack_ [~jon@37.173.103.51] has joined #bitcoin-core-pr-reviews 00:32 -!- jonatack [~jon@37.170.227.51] has quit [Ping timeout: 240 seconds] 00:40 -!- mol_ [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 00:41 -!- vasild_ [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 00:44 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 00:44 -!- vasild_ is now known as vasild 00:45 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 260 seconds] 01:40 -!- Jackielove4u_ [uid43977@gateway/web/irccloud.com/x-bgntuuwpekckguoj] has joined #bitcoin-core-pr-reviews 01:41 -!- Jackielove4u_ [uid43977@gateway/web/irccloud.com/x-bgntuuwpekckguoj] has quit [Client Quit] 01:41 -!- Jackielove4u_ [uid43977@gateway/web/irccloud.com/x-ultqhrhlbjxkeltu] has joined #bitcoin-core-pr-reviews 01:42 -!- Jackielove4u_ [uid43977@gateway/web/irccloud.com/x-ultqhrhlbjxkeltu] has quit [Client Quit] 01:42 -!- Jackielove4u [uid43977@gateway/web/irccloud.com/x-lrnsjvfkedukqqhl] has joined #bitcoin-core-pr-reviews 02:24 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:b865:43b9:34cd:d980] has joined #bitcoin-core-pr-reviews 02:24 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:b865:43b9:34cd:d980] has quit [Client Quit] 02:38 -!- jungly [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has quit [Read error: Connection reset by peer] 02:38 -!- jungly_ [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has joined #bitcoin-core-pr-reviews 02:38 -!- jungly_ [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has quit [Read error: Connection reset by peer] 02:39 -!- jungly [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has joined #bitcoin-core-pr-reviews 02:43 -!- jungly [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has quit [Ping timeout: 260 seconds] 02:51 -!- brakmic [~brakmic@ip-176-198-41-116.hsi05.unitymediagroup.de] has joined #bitcoin-core-pr-reviews 03:03 -!- Alvera16Kovacek [~Alvera16K@ns334669.ip-5-196-64.eu] has joined #bitcoin-core-pr-reviews 03:06 -!- jungly [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has joined #bitcoin-core-pr-reviews 03:08 -!- Alvera16Kovacek [~Alvera16K@ns334669.ip-5-196-64.eu] has quit [Ping timeout: 240 seconds] 03:15 -!- jonatack__ [~jon@37.172.252.66] has joined #bitcoin-core-pr-reviews 03:18 -!- jonatack_ [~jon@37.173.103.51] has quit [Ping timeout: 260 seconds] 04:00 -!- jonatack__ [~jon@37.172.252.66] has quit [Read error: Connection reset by peer] 04:01 -!- jonatack__ [~jon@37.172.252.66] has joined #bitcoin-core-pr-reviews 05:33 -!- jonatack__ [~jon@37.172.252.66] has quit [Quit: jonatack__] 05:34 -!- jonatack [~jon@37.172.252.66] has joined #bitcoin-core-pr-reviews 05:43 -!- theStack [~theStack@81.223.165.6] has joined #bitcoin-core-pr-reviews 05:44 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:c828:aba9:8f63:78f3] has joined #bitcoin-core-pr-reviews 05:44 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:c828:aba9:8f63:78f3] has quit [Client Quit] 06:04 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 06:06 -!- mol_ [~molly@unaffiliated/molly] has quit [Ping timeout: 256 seconds] 06:34 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 258 seconds] 06:48 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 06:59 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 260 seconds] 07:04 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 07:16 -!- theStack [~theStack@81.223.165.6] has quit [Remote host closed the connection] 07:17 -!- theStack [~theStack@81.223.165.6] has joined #bitcoin-core-pr-reviews 07:23 -!- theStack [~theStack@81.223.165.6] has quit [Remote host closed the connection] 07:26 -!- theStack [~honeybadg@vps1648322.vs.webtropia-customer.com] has joined #bitcoin-core-pr-reviews 07:34 -!- theStack [~honeybadg@vps1648322.vs.webtropia-customer.com] has quit [Quit: leaving] 07:36 -!- theStack [~honeybadg@vps1648322.vs.webtropia-customer.com] has joined #bitcoin-core-pr-reviews 07:57 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews 08:16 -!- Elle [9b5dfc46@155.93.252.70] has joined #bitcoin-core-pr-reviews 08:19 -!- Elle [9b5dfc46@155.93.252.70] has quit [Remote host closed the connection] 08:27 -!- jungly [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has quit [Read error: Connection reset by peer] 08:27 -!- jungly_ [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has joined #bitcoin-core-pr-reviews 08:53 -!- Talkless [~Talkless@hst-227-49.splius.lt] has joined #bitcoin-core-pr-reviews 09:06 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 240 seconds] 09:13 -!- as_pnn [~pierreirc@119.192.247.147] has joined #bitcoin-core-pr-reviews 09:15 -!- kristapsk [~KK@gateway/tor-sasl/kristapsk] has quit [Ping timeout: 240 seconds] 09:16 -!- lightlike [~lightlike@p200300C7EF10B5001179462A28E4675A.dip0.t-ipconnect.de] has joined #bitcoin-core-pr-reviews 09:20 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews 09:22 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Ping timeout: 240 seconds] 09:23 -!- shaunsun [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has joined #bitcoin-core-pr-reviews 09:23 -!- shaunsun_ [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has joined #bitcoin-core-pr-reviews 09:25 -!- shaunsun__ [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has joined #bitcoin-core-pr-reviews 09:27 -!- andrewtoth_ [~andrewtot@gateway/tor-sasl/andrewtoth] has quit [Remote host closed the connection] 09:28 -!- shaunsun [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has quit [Ping timeout: 256 seconds] 09:28 -!- shaunsun_ [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has quit [Ping timeout: 260 seconds] 09:45 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews 09:50 -!- jannes [~jannes@095-097-246-234.static.chello.nl] has joined #bitcoin-core-pr-reviews 09:51 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:c828:aba9:8f63:78f3] has joined #bitcoin-core-pr-reviews 09:55 < jnewbery> We'll get started in 5 minutes 09:58 < MarcoFalke> We'll get started in 2 minutes 09:58 < jnewbery> Thanks Marco! 09:59 -!- shaunsun__ [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has quit [Quit: Leaving] 09:59 -!- shaunsun__ [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has joined #bitcoin-core-pr-reviews 09:59 < MarcoFalke> #startmeeting 10:00 < MarcoFalke> hi everyone 10:00 < lightlike> hi 10:00 < theStack> hi 10:00 < andrewtoth> hi 10:00 < michaelfolkson> hi 10:00 -!- robot-visions [ac5c056c@172.92.5.108] has joined #bitcoin-core-pr-reviews 10:00 < sipa> hi 10:01 < shaunsun__> hi 10:01 < vasild> hi 10:01 < brikk> hi 10:01 < robot-visions> hi 10:01 < MarcoFalke> Reminder that you are welcome to ask questions. Don't ask to ask. We are all here to learn (including myself) 10:01 < emzy> Hi 10:01 < rjected> hi 10:01 < jnewbery> hi 10:01 < MarcoFalke> To get started, did you get a chance to review the pull request or take a look at it? 10:01 -!- soup [47c4aa57@c-71-196-170-87.hsd1.co.comcast.net] has joined #bitcoin-core-pr-reviews 10:01 < nehan_> hi 10:02 < robot-visions> (y) reviewed 10:02 < nehan_> y 10:02 < vasild> y, briefly 10:02 < emzy> n 10:02 < soup> n 10:02 < lightlike> yes 10:02 < andrewtoth> y 10:02 < theStack> n (still occupied with #17860) 10:02 < jonatack> hi 10:03 < michaelfolkson> Took a look at it yes. Trying to learn more about fuzz testing generally so I can better understand where we are going with it. 10:03 -!- chanho [b8980f30@cpe-184-152-15-48.nyc.res.rr.com] has joined #bitcoin-core-pr-reviews 10:03 < MarcoFalke> ok, so to get started with the net_processing vulnerability in the bloom filter that the fuzz tests is trying to find. What are the messages a remote node needs to send? 10:04 < robot-visions> Send a `filterload` where the bloom filter's underlying array is empty, then send a `filteradd` with arbitrary data; see [#18515](https://github.com/bitcoin/bitcoin/pull/18515) 10:04 < MarcoFalke> michaelfolkson: Good to hear. I am hosting this club to get everyone introduced to fuzzing :) 10:04 < MarcoFalke> robot-visions: Correct 10:04 < MarcoFalke> Did anyone come up with alternative ways to trigger the vulnerability? 10:05 < michaelfolkson> This was good. Any other good recommended resources would be gratefully received https://diyhpl.us/wiki/transcripts/cppcon/2017/2017-10-11-kostya-serebryany-fuzzing/ 10:05 < robot-visions> Is it necessary to send the second `filteradd`, or is it enough to just wait for an `inv` from elsewhere? 10:05 < jonatack> michaelfolkson: you'll find the PR description most helpful :p 10:05 < michaelfolkson> Yup that was good too ;) 10:05 -!- frequency [~bubble@185.180.13.45.adsl.inet-telecom.org] has joined #bitcoin-core-pr-reviews 10:06 < MarcoFalke> robot-visions: Anything that eventually calls CBloomFilter::Hash should crash the node 10:07 < robot-visions> Thanks MarkoFalke! Makes sense. 10:07 < MarcoFalke> So I believe waiting for the node to process a transaction either from itself or from another peer should also hit the crash 10:07 < MarcoFalke> How was this vulnerability fixed? 10:07 < michaelfolkson> So practicalswift and robot-visions were able to trigger it. They triggered it in different ways? 10:08 < MarcoFalke> michaelfolkson: I haven't looked at their fuzz inputs, but we can do that later 10:08 < michaelfolkson> Ok 10:08 < MarcoFalke> (If they share them) 10:09 < robot-visions> Vulnerability was fixed by introducing `isEmpty` and `isFull` flags to return early if the Bloom filter's underlying array is empty; see [#2914](https://github.com/bitcoin/bitcoin/pull/2914) 10:10 < lightlike> It was fixed covertly, by adding a UpdateEmptyFull() function that is processed on filterload (net_processing) an sets isFull if the underlaying array is empty. Filteradd will not insert anything if isFull is set 10:10 < theStack> is there plans to commit certain fuzz inputs into the repository? (or is that already the case?) 10:10 < MarcoFalke> theStack: They are located in bitcoin-core/qa-assets on GitHub 10:11 < theStack> MarcoFalke: ah, i see. tend to forget that there are also other repos outside of bitcoin/bitcoin ;-) 10:11 < MarcoFalke> robot-visions: lightlike: Correct 10:12 < lightlike> I understand why it was fixed this way back then, but wouldn't it make sense to do something more obvious today (like also check for zero in Hash()?) 10:12 < theStack> i was quite fascinated by this covert fix. i can imagine is it really hard to fix something but needing to pretend doing something different 10:12 < theStack> lightlike: the point is, if you do it in an obvious way, the vulnerability becomes public and is exploited 10:13 < chanho> I think the question is, should the covert fix be removed and updated with a proper fix, e.g. check for zero division? 10:13 < lightlike> theStack: yes, I understand, but my question referred to today where basically all nodes are no longer vulnerable 10:14 < MarcoFalke> I think theStack has a good point. If the fix for this trivial exploit is a one-line patch, it might make it easy for bad players to exploit it 10:15 < theStack> chanho: lightlike: to my understanding, it could be removed and done in a simpler way 10:16 < jnewbery> lightlike chanho: I agree that after some time has passed (perhaps a couple of years) these covert fixes should be cleared up and made public. Leaving them in goes against the goal of having the code as clear and straightforward as possible. 10:17 < MarcoFalke> I haven't thought about this question upfront, but I think even today the patch would look similar. 10:17 < jnewbery> to be fair, I don't think they're left in this way on purpose. I expect the person who fixes it probably just forgets and it's not their priority after two years 10:18 < MarcoFalke> But let's not talk about how the code can be cleaned up today and focus on fuzzing :) 10:18 < MarcoFalke> All we need to know about the vulnerability for now is how it was triggered and how it was fixed 10:18 < michaelfolkson> The fuzz inputs generated here. How were they generated? Much thought go into it? https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus 10:19 < MarcoFalke> michaelfolkson: Anyone can generate them, but we'll cover this in the second half of the meeting 10:19 < michaelfolkson> Ok sorry :) 10:19 < MarcoFalke> no worries 10:19 < MarcoFalke> Let's take a look at the structure of Bitcoin Core. Where in CNode are the bloom filters stored? 10:20 < robot-visions> I think it's in `CNode->m_tx_relay->pfilter` 10:20 < MarcoFalke> Right 10:20 < theStack> robot-visions: +1 10:20 < MarcoFalke> And where are bloom filter messages handled? 10:21 < theStack> net_processing.cpp, directly in the huge ProcessMessage() function 10:21 < robot-visions> theStack: (y) 10:23 < MarcoFalke> A fuzzing harness needs an entry point, and to execute the bloomfilter code, it needs to call into the ProcessMessage function 10:24 < MarcoFalke> Does anyone have questions about the structure of the fuzzing harness? 10:24 -!- jungly_ [~jungly@host21-196-dynamic.246-95-r.retail.telecomitalia.it] has quit [Remote host closed the connection] 10:25 < MarcoFalke> Pretty much what it does is, (1) create a few CNode (with the bloomfilters initialized), then (2) pretend they are sending random messages by passing them into the ProcessMessage function 10:26 < sipa> jnewbery: an alternative is (in general) not removing covert fixes (because touching the code again introduces risks of its own), but still documenting them in the code 10:26 < andrewtoth> What is the purpose of having a random number of nodes created? Is it because it affects what they do with the messages if they have different number of peers? 10:26 -!- LarryRuane [62f5cc94@c-98-245-204-148.hsd1.co.comcast.net] has joined #bitcoin-core-pr-reviews 10:27 < michaelfolkson> I suppose you are receiving different numbers of messages depending on how many peers you have 10:28 < MarcoFalke> sipa: Good point . I've done something for the money printing bug in #17080 10:29 < theStack> If a constant number of nodes were be sufficient, could that CNode creation code move into initialize()? Or is there any other reason it has to be created again and again for each input? 10:29 < theStack> s/were be/were/ 10:29 < MarcoFalke> andrewtoth: Good question. The purpose of the fuzz test was to be more than just a test to find the bloomfilter bug. It should also test other parts of the code like transaction relay, which has a lot of state 10:30 < lightlike> theStack: I believe the service flags are fuzzed too, so maybe that is the reason? 10:30 < MarcoFalke> Unfortunately, the current fuzzer can not test transaction relay because messages are constructed from something that looks like a random stream 10:31 < theStack> lightlike: you are right, there are numerous other properties that are fuzzed 10:31 < MarcoFalke> lightlike: Jup, I tried to be as permissive as possible. Give the fuzz test as much space to search as possible. 10:31 < andrewtoth> MarcoFalke: so to test transaction relay, a separate harness that uses the fuzz inputs to generate different relay messages needs to be built? 10:32 < lightlike> I noticed that the fuzzer needs quite some of tries (>10k) until it gets to the ProcessMessage part the first time 10:32 < sipa> MarcoFalke: a special fuzzregtest mode that disables signature validation? 10:32 < MarcoFalke> andrewtoth: In theory the fuzz engine could figure out how to construct transactions. All it has to do is guess the inputs correctly 10:32 < MarcoFalke> sipa: It needs to guess the hash of the input 10:33 < MarcoFalke> Which has low probability assuming the best you can do is random guesses 10:33 < jnewbery> sipa: (re removing covert fixes) yes. I think it depends on context (riskiness of change vs benefit of clarifying code to match intent) 10:34 < andrewtoth> So a harness which better guides the fuzzer would be more efficient for tx relay 10:34 < sipa> MarcoFalke: oh, right 10:34 < MarcoFalke> However, modern fuzz engines do a lot better on many things than just random guesses. For example, they can extract strings easily 10:35 < MarcoFalke> You can see that when running the fuzzer. In a few seconds it has already guessed some of the message types 10:35 < MarcoFalke> andrewtoth: Yes 10:35 < MarcoFalke> I am still thinking what the best way is to achieve this 10:35 < sipa> right, because the coverage guiding can observe the characters being compared against 10:35 < sipa> this wouldn't be the case for a hash comparison 10:36 < theStack> MarcoFalke: would you count both libfuzzer and AFL to the category of "modern fuzz engines"? or are there better ones (commercial or anything)? 10:36 < MarcoFalke> sipa: Yes. I don't know how fuzz engines work, but I could also imagine that it might dump the binary and see if there are any strings in it. 10:37 < sipa> MarcoFalke: i believe not 10:37 < MarcoFalke> (Not sure if you can even inspect your own binary with C++) 10:38 < MarcoFalke> theStack: I have only tried the two 10:38 < lightlike> I thought it is some kind of genetic algorithm that tries to optimize code coverage and performs all kind of mutations. 10:38 < MarcoFalke> So did anyone find the crasher? What patch did you have to apply to re-introduce the vulnerability? 10:38 < michaelfolkson> theStack: For fuzzing open source projects, libFuzzer and AFL were modern in 2017 10:39 < robot-visions> To re-introduce the vulnerability: It suffices to remove the `if (isFull) return` check at the beginning of `CBloomFilter::insert` 10:39 < lightlike> I noticed the fuzzer finds easy messages ("inv") much faster than longer ones ("filterload") 10:40 < MarcoFalke> For me the first thing it found was -dropmessagetest 10:40 < nehan_> i'm running the fuzzer but it doesn't seem to be showing me which messages it's using: 10:40 < MarcoFalke> Which is a command line flag, ouch 10:40 < lightlike> nehan: i grepped the corpus directory for the strings to see if they were found yet 10:41 < MarcoFalke> nehan_: If you use libfuzzer, sometimes it will tell you in the stdout lines which "dictionary" words were added or removed 10:41 < MarcoFalke> It is also possible to inspect the input directory directly 10:41 < sipa> MarcoFalke: are you familiar with the -dict optionm 10:41 < sipa> ? 10:41 < MarcoFalke> Though, be warned if you use cat to redirect a fuzz input to your terminal 10:41 < nehan_> lightlike: thanks 10:41 < nehan_> MarcoFalke: I'm wondering if I didn't compile something correctly, I only seem to have symbols: 10:41 < nehan_> #868056 REDUCE cov: 17995 ft: 84042 corp: 1298/1621Kb exec/s: 446 rss: 510Mb L: 3483/4096 MS: 4 InsertByte-InsertByte-InsertRepeatedBytes-EraseBytes- 10:42 < MarcoFalke> sipa: no, is that an option to libfuzzer? 10:42 < sipa> nehan_: it lists the mutations it is applying to existing seeds 10:42 < MarcoFalke> nehan_: It shows it only rarely for me 10:42 < sipa> nehan_: if it finds a crash, it will create a file with the input that causes the crash 10:43 < sipa> MarcoFalke: yeah, to tell it about strings that are likely relevant to your fuzzer 10:43 < jonatack> nehan_: i'm seeing the same, and occasionally there's a message at the end of the line 10:43 < jonatack> 371324REDUCE cov: 14900 ft: 65908 corp: 907/963Kb exec/s: 388 rss: 534Mb L: 2020/4096 MS: 5 CMP-ChangeBinInt-InsertRepeatedBytes-InsertRepeatedBytes-EraseBytes- DE: "verack"- 10:43 < MarcoFalke> sipa: Ah, so I could pass in the utxo set as a dict? 10:44 < MarcoFalke> jonatack: Thanks, that is what I meant 10:44 < sipa> MarcoFalke: well for that you could also just manually create a seed 10:44 < jonatack> nehan_: (good question) 10:44 < sipa> i believe DE is in fact it using a dictionary word 10:44 < nehan_> i've been running for an hour or so and it has created 1330 files. All of those can't be crashes? 10:44 < sipa> nehan_: only if they're called crash-XXXXX 10:45 < nehan_> sipa: ah, thanks. let me go learn more about libfuzzer... 10:45 < MarcoFalke> If you inspect the seeds manually with cat, please pass in `cat --show-nonprinting`, otherwise it might execute arbitary code in your terminal 10:45 < sipa> the files you usually see are the seeds 10:46 < sipa> they're the input that maximize coverage (of code and of "features") 10:46 < nehan_> how long did it take folks to find the crash? 10:46 < MarcoFalke> To run the fuzz harness can supply a folder where to put the seeds: ./src/test/fuzz/process_messages ./where/to/put/the/seeds 10:46 < MarcoFalke> And then `cat --show-nonprinting ./where/to/put/the/seeds/000fff...` 10:47 < MarcoFalke> nehan_: Good question. I am also interesed in that. 10:47 < lightlike> I noticed if I stop the fuzzer (Ctrl+C) and start it again with the same seed dir, the coverage will be significantly lower than before, as if it didn't reload all the work done before. Does anyone know why this is the case? (I thought the seeds would be updated after each try) 10:48 < MarcoFalke> I think the fastest I got was 600k, but with 9 workers in parallel, so I am not sure if that counts because you can't compare it against a single worker. (The libfuzzer workers work together on the same seed corpus) 10:49 < jonatack> I'm still running it, will comment in the PR when it crashes 10:49 < sipa> lightlike: as in the "cov" number is lower? 10:49 < sipa> that'd surprise me 10:49 < lightlike> sipa: yes, cov and ft 10:49 < MarcoFalke> lightlike: Good question. I believe it is because of "adjacant" coverage. Bitcoin core runs a lot of threads (like the stale tip check in the scheduler), and those are not included in the coverage if you start afresh 10:49 < sipa> ah 10:50 < MarcoFalke> Which is unfortunate, but I don't know how to "fix" 10:50 < sipa> could be the result of non-deterministic behavior 10:50 < MarcoFalke> sipa: Or that of course 10:50 < sipa> if the fuzzed binary uses randomness anywhere, fuzzing is less.efficient 10:50 < MarcoFalke> I think my greatest fear would be to find a crasher and then try to reproduce it with the same seed and it wouldn't crash because of non-determinism 10:51 < MarcoFalke> deterministic randomness is fine (as long as it is derived by the fuzz engine) 10:51 < lightlike> I didn't find the bug after 10M steps and stoppped, but found it after 2.2 million steps with a version in which I replaced the MsgType Fuzzing with an array of all known Messages, for which then only the index is fuzzed. 10:52 < MarcoFalke> Did anyone run into issues while fuzzing or setting up the fuzzer? 10:52 < emzy> The fuzzing is deterministic? So generated by a pseudorandom number generator? 10:53 < robot-visions> MarkoFalke: I was able to get things work by following the "macOS hints for libFuzzer", but I had to remove the `--disable-asm` flag when running `./configure` 10:53 < robot-visions> Marco* (sorry) 10:53 < MarcoFalke> emzy: I think the fuzz engine picks a random seed at the beginning, but it might be possible to make even the fuzzing deterministic 10:54 < emzy> I see. 10:54 < sipa> you can pick the rng seed 10:54 < MarcoFalke> robot-visions: Is that wrong in our docs? It could help others if you amend that note there 10:56 < MarcoFalke> sipa: Correct, for libfuzzer you can pick the internal starting seed 10:56 < theStack> MarcoFalke: i had a linking issue with the http_request fuzz test, which used internal libevent functions (evhttp_parse_firstline_) that were named slightly different on my libevent -- could fix it locally though, will open an issue later 10:56 < MarcoFalke> theStack: Does Bitcoin Core compile fine without libFuzzer? 10:57 < theStack> MarcoFalke: yes it does 10:57 < MarcoFalke> hm 10:58 < nehan_> i had an unused variable warning in test/fuzz/locale.cpp:59 10:58 < jonatack> sipa: MarcoFalke: found this on the -dict option sipa mentioned https://llvm.org/docs/LibFuzzer.html#dictionaries 10:58 < jonatack> full list of options here: https://llvm.org/docs/LibFuzzer.html#options 10:58 < andrewtoth> nehan_ same 10:58 < robot-visions> MarcoFalke: I think the docs are reasonable right now. It says (1) "you may need to run `--disable-asm` to avoid errors, and (2) "here's what worked on our setup (which included `--disable-asm`)". It doesn't say you *must* use that flag. 10:59 < MarcoFalke> nehan_: Good point. The warning is caused because a fuzz test was modified. To not invalidate existing seeds, the fuzz harness still needs to consume the same bytes it did previously, but now they are unused. I think this can be fixed by prefixing the line with (void) 10:59 < nehan_> MarcoFalke: interesting! 11:00 < MarcoFalke> #endmeeting 11:00 < MarcoFalke> Thanks everyone! 11:00 < robot-visions> Thanks! 11:00 < vasild> Thanks! 11:00 < theStack> thanks for hosting! 11:00 < nehan_> thanks! 11:00 < jonatack> or passing -help=1 ... e.g. src/test/fuzz/process_messages -help=1 11:00 < emzy> Thanks! 11:00 < jonatack> thanks MarcoFalke! 11:01 < lightlike> thanks! 11:01 < nehan_> i've never done any fuzzing before so this was cool 11:02 < andrewtoth> Thanks MarcoFalke! 11:02 -!- soup [47c4aa57@c-71-196-170-87.hsd1.co.comcast.net] has quit [Remote host closed the connection] 11:03 < jnewbery> thanks MarcoFalke. Great Review Club! 11:03 < michaelfolkson> You going to hang around for a bit MarcoFalke or you need to go? 11:03 < jonatack> If useful, MarcoFalke also hosted a review club session on fuzzing in January: https://bitcoincore.reviews/17860 11:04 < theStack> to shortly bring up the subject of remaining covert fixes again: it can be confusing to code readers if it remains. E.g. it led to the issue #16886 and its fix PR #16922 (the latter one by me), both made in the wrong assumption that the point of the empty/full flags were optimization, as we didn't know about the covert fix 11:04 < jonatack> theStack: the meeting log for that session begins with us trying to get fuzz/test_runner.py to work :) 11:05 < robot-visions> On a similar note to what theStack mentioned, would it make sense to now consider a peer "misbehaving" if they send an empty bloom filter? 11:05 < theStack> jonatack: awesome -- though from my side it was more of a conclusion that i don't need the fuzz test_runner ;) 11:05 < jonatack> theStack: i agree that a pr like https://github.com/bitcoin/bitcoin/pull/17080 that documents this could be helpful 11:07 < theStack> robot-visions: personally i think that would be fine, don't know though how strict BIP37 is interpreted -- it afair only defines an upper filter size limit, but not a lower limit 11:08 -!- chanho [b8980f30@cpe-184-152-15-48.nyc.res.rr.com] has quit [Remote host closed the connection] 11:08 < theStack> robot-visions: on the other hand i wouldn't know the point of an empty filter 11:09 < theStack> jonatack: yes i also think that a short explanation with CVE number mentioning would be more appropriate 11:09 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 11:12 < jonatack> robot-visions: interesting question 11:13 < lightlike> theStack: I found it interesting that I didn't find any detailed description on how to crash the node using the CVE apart from the one in your PR. Did you find any, or did you just figure it out yourself? 11:17 < theStack> lightlike: i was also wondering the same. the only information i got was that it is triggered by a division by zero. then i inspected the CBloomFilter class for divisions, and since the class is not that large i could figure it out quite fast 11:19 < theStack> now at least in the bitcoin-wiki about CVEs both the covert fix and were linked by someone (https://en.bitcoin.it/wiki/Common_Vulnerabilities_and_Exposures#CVE-2013-5700) 11:19 < andrewtoth> I think the exploit is not described in detail anywhere conspicuous intentially 11:19 < andrewtoth> *intentionally 11:19 < sipa> at the time, it was certainly intentionally 11:19 < sipa> though it should be documented now 11:20 < andrewtoth> sipa: +1 11:20 < lightlike> andrewtoth: I thought that whenever the CVE is published (in the wiki/mailing list) everything will be made public. 11:21 < sipa> lightlike: it's a very ad-hoc process really 11:23 < robot-visions> Thanks again everyone! I haven't seen fuzzing before, it's really interesting. Hope to see you at a future session. 11:23 -!- robot-visions [ac5c056c@172.92.5.108] has quit [Remote host closed the connection] 11:23 < theStack> interestingly enough there still seem to be listening nodes that are vulnerable to this (looking at the user agents on bitnodes.io) 11:24 -!- mol_ [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 11:24 < emzy> Also altcoins may be on old versions. 11:25 < sipa> theStack: there are still nodes with code from 2013? :s 11:25 < emzy> But i think there was enought time to update for both. 11:27 < MarcoFalke> I think the issue with making CVEs public is mostly people forgetting about them 11:27 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 250 seconds] 11:28 < sipa> yeah 11:28 < MarcoFalke> Obviously you can't make them public on the first day they are discovered, and if the time has passed to make them public, they might be forgotten about 11:29 < theStack> sipa: well according to that site five nodes still are on 0.8.x 11:29 < theStack> x >= 5 though... this CVE was fixed in 0.8.4, so they are at least not vulnerable to this 11:30 < lightlike> I'd guess the finder would like attribution - even if there are no bounties, finding a CVE in core seems like something to be proud of. 11:31 < lightlike> at least if they are not a regular 11:32 < MarcoFalke> Everyone who reports an issue to the security email of Bitcoin Core will get mentioned in the release notes of the release that fixed it 11:35 -!- as_pnn [~pierreirc@119.192.247.147] has quit [Read error: Connection reset by peer] 11:39 < jonatack> About the "DE:" keys that prefix new messages in the fuzzer output, https://llvm.org/docs/LibFuzzer.html#output doesn't mention them, my guess is they might mean Dictionary Entry? 11:40 < sipa> that's my guess 11:40 -!- kristapsk [~KK@gateway/tor-sasl/kristapsk] has joined #bitcoin-core-pr-reviews 11:44 < lightlike> Internally to the fuzzer, there is also "PersAutoDict" that you see in the output: "Persistent dictionary modified by the fuzzer, consists entries that led to successfull discoveries in the past mutations." 11:44 -!- theStack [~honeybadg@vps1648322.vs.webtropia-customer.com] has quit [Quit: Lost terminal] 12:01 < jonatack> yes, in the mutation operations output, e.g. MS: 3 ChangeBit-PersAutoDict-EraseBytes- DE: "\xdf\... 12:02 < jonatack> lightlike: thanks, what doc are you quoting from? 12:03 < lightlike> jonatack: I googled for PersAutoDict, and this is a code comment from libfuzzer 12:05 < lightlike> https://llvm.org/doxygen/FuzzerMutate_8h_source.html 12:06 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:c828:aba9:8f63:78f3] has quit [Quit: Sleep mode] 12:09 < jonatack> lightlike: thanks -- the code confirms that DE is for Dictionary Entry too 12:10 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:c828:aba9:8f63:78f3] has joined #bitcoin-core-pr-reviews 12:10 -!- michaelfolkson [~textual@2a00:23c5:be01:b201:c828:aba9:8f63:78f3] has quit [Client Quit] 12:21 -!- jules19 [2578d0ee@37.120.208.238] has joined #bitcoin-core-pr-reviews 12:22 -!- jules19 [2578d0ee@37.120.208.238] has quit [Remote host closed the connection] 12:26 -!- masterdonx2 [~masterdon@144.48.39.84] has quit [Quit: ZNC 1.7.5 - https://znc.in] 12:27 -!- MasterdonX [~masterdon@144.48.39.84] has joined #bitcoin-core-pr-reviews 12:28 -!- Talkless [~Talkless@hst-227-49.splius.lt] has quit [Quit: Konversation terminated!] 12:31 -!- frequency [~bubble@185.180.13.45.adsl.inet-telecom.org] has left #bitcoin-core-pr-reviews [] 12:33 < instagibbs> another example of covert fix being documented but not touched otherwise: https://github.com/bitcoin/bitcoin/pull/16885 12:44 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 12:46 -!- vasild [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 13:03 -!- vasild [~vd@gateway/tor-sasl/vasild] has quit [Ping timeout: 240 seconds] 13:05 -!- vasild [~vd@gateway/tor-sasl/vasild] has joined #bitcoin-core-pr-reviews 13:29 -!- grunch__ [~grunch@78-170-16-190.fibertel.com.ar] has joined #bitcoin-core-pr-reviews 13:44 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has quit [Remote host closed the connection] 13:58 -!- jb55 [~jb55@gateway/tor-sasl/jb55] has joined #bitcoin-core-pr-reviews 13:58 -!- grunch__ [~grunch@78-170-16-190.fibertel.com.ar] has quit [Ping timeout: 260 seconds] 14:04 -!- jonatack [~jon@37.172.252.66] has quit [Ping timeout: 260 seconds] 14:06 -!- jonatack [~jon@37.171.222.247] has joined #bitcoin-core-pr-reviews 14:08 -!- shaunsun__ [shaunsun@gateway/vpn/privateinternetaccess/shaunsun] has quit [Quit: Leaving] 14:30 -!- kristapsk [~KK@gateway/tor-sasl/kristapsk] has quit [Remote host closed the connection] 14:30 -!- kristapsk [~KK@gateway/tor-sasl/kristapsk] has joined #bitcoin-core-pr-reviews 14:55 -!- jonatack [~jon@37.171.222.247] has quit [Ping timeout: 265 seconds] 14:56 -!- jonatack [~jon@213.152.162.69] has joined #bitcoin-core-pr-reviews 14:59 -!- LarryRuane [62f5cc94@c-98-245-204-148.hsd1.co.comcast.net] has quit [Remote host closed the connection] 15:52 -!- provoostenator [~quassel@provoostenator.sprovoost.nl] has quit [Remote host closed the connection] 15:54 -!- provoostenator [~quassel@provoostenator.sprovoost.nl] has joined #bitcoin-core-pr-reviews 16:32 -!- brakmic [~brakmic@ip-176-198-41-116.hsi05.unitymediagroup.de] has quit [] 17:12 -!- mol_ [~molly@unaffiliated/molly] has quit [Ping timeout: 258 seconds] 17:13 -!- mol [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 18:32 -!- mol_ [~molly@unaffiliated/molly] has joined #bitcoin-core-pr-reviews 18:33 -!- lightlike [~lightlike@p200300C7EF10B5001179462A28E4675A.dip0.t-ipconnect.de] has quit [Remote host closed the connection] 18:33 -!- mol [~molly@unaffiliated/molly] has quit [Ping timeout: 265 seconds] 20:36 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has quit [Quit: Quit] 20:36 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has joined #bitcoin-core-pr-reviews 20:37 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has quit [Client Quit] 20:37 -!- BlueMatt [~BlueMatt@unaffiliated/bluematt] has joined #bitcoin-core-pr-reviews 21:49 -!- Talkless [~Talkless@hst-227-49.splius.lt] has joined #bitcoin-core-pr-reviews 21:50 -!- Talkless [~Talkless@hst-227-49.splius.lt] has quit [Client Quit] 22:08 -!- robot-visions [ac5c056c@172.92.5.108] has joined #bitcoin-core-pr-reviews 22:15 -!- robot-visions [ac5c056c@172.92.5.108] has quit [Remote host closed the connection] 22:29 -!- MarcoFalke [~none@198.12.116.246] has quit [Read error: Connection reset by peer] 22:30 -!- MarcoFalke [~none@198.12.116.246] has joined #bitcoin-core-pr-reviews 23:36 -!- gwillen [~gwillen@unaffiliated/gwillen] has quit [Ping timeout: 240 seconds] 23:37 -!- gwillen [~gwillen@unaffiliated/gwillen] has joined #bitcoin-core-pr-reviews