search

davmac314 / dinit

Service monitoring / "init" system
Apache License 2.0
582 stars 45 forks source link

control: Don't call process_packet() when bad_conn_closed is set #354

Closed mobin-2008 closed 1 month ago

mobin-2008 commented 1 month ago

CWE-835: Loop with Unreachable Exit Condition ('Infinite Loop')

Sending garbage data and then closing the connection could lead into out-of-memory. Because after closing the connection, Dinit will stuck in (rbuf.get_length() >= chklen) while loop and then the outbuf buffer is used because the connection fd is not available anymore. This triggers the OOM detector in the fuzzer and can result in segfault when Dinit is used as init of system.

It's mostly harmless because triggering it requires root or particular user (the user who starts dinit) access. See https://github.com/davmac314/dinit/blob/master/doc/SECURITY

Firstly detected in https://github.com/davmac314/dinit/actions/runs/7233843212

Signed-off-by: Mobin Aydinfar <mobin@mobintestserver.ir>

mobin-2008 commented 1 month ago

@davmac314 I also started a fuzzer to make sure this problem is resolved: https://github.com/mobin-2008/dinit/actions/runs/10028795810/job/27716167886. I will request review when it succeeded.

I will write and sent the test about this scenario in separate PR.

davmac314 commented 1 month ago

I will write and sent the test about this scenario in separate PR.

Let's just wait and get the test in the same PR. That's the usual process, and it ensures that the test does not get forgotten.

Thanks for this, the change itself is fine.

mobin-2008 commented 1 month ago

I will write and sent the test about this scenario in separate PR.

Let's just wait and get the test in the same PR. That's the usual process, and it ensures that the test does not get forgotten.

I wasn't sure that I could have enough time for it but I have now. I was thinking about more tests about this issue but I think the more testing is the purpose of fuzzer not cptests (For example passing a valid command following with an invalid command).

Test is ready for review.