CaringCaribou / caringcaribou

A friendly car security exploration tool for the CAN bus
GNU General Public License v3.0
751 stars 197 forks source link

UDS Seed Randomness Fuzzer and Delay Fuzzer #70

Closed Cr0wTom closed 2 years ago

Cr0wTom commented 3 years ago

Research showed that many manufacturers do not seed with enough entropy the seed/key algorithm of modern ECUs. In more detail (source):

the random number generator is seeded with the system timer, which is not a source of entropy because it behaves predictably

There is also a complete research done regarding this specific topic by Sekar Kulandaivel.

After proving the vulnerability in real life testing, the need for an automated tool was apparent. In the uds_fuzz module, I implemented the following two sub-functions:

Cr0wTom commented 3 years ago

Thanks for your review and comments @kasperkarlsson .

The module got developed due to real world use and needs that couldn't been covered by cc's current functionality. Clearly I am not a developer, and the whole thing started as a standalone project, but I want to help the community and I guess through cc it would have greater impact (and I support the modular nature of cc of course).

As you mentioned, many parts are taken from the uds module. One of the ideas was to extend the uds module, but I didn't want to break anything. Maybe I can move the functions there, if that's preferable, but the way the sessions are supplied, parsed and handled is completely different from the UDS module (in order to fix issues I had with uds module, like this).

As you can see, I fixed the proposed issues, so let me know if there is a different way forward that is preferred, or if some references are needed inside the code, pointing to the UDS module. :)

Cr0wTom commented 2 years ago

Thanks for the response once again.

I fixed the proposed issues and the problematic usage example.

Also I made a change to the message sequence example. Before, in the example requests I included the different requests with the data size and the padding and then stripped those two attributes out. Now I have a more descriptive approach that I hope is more clear.

kasperkarlsson commented 2 years ago

Excellent! Should the message sequence example be modified in the help output for delay_fuzzer as well? After that, I believe we are ready for a merge 😁

Cr0wTom commented 2 years ago

Perfect, it's done :D Thanks for your help!

kasperkarlsson commented 2 years ago

Thank you so much for your contribution (and for enduring my typo/import nagging 😅 I will make some minor adjustments for PEP 8 compliance (i.e. just formatting - no functionality changes) shortly.

Merged and done - thanks again and have a great day! 💪

Cr0wTom commented 2 years ago

Thank you too, no worries at all. Should I later also update/populate the README and Documentation?

kasperkarlsson commented 2 years ago

Sure thing - if you want to update the README and documentation, feel free to send a PR for that as well!