Open PeterSurda opened 6 years ago
1 The project uses pickle. Pickle is insecure and can be used to inject code. Anyone with permissions to write into these files can inject code. It is unexpected and can be used as a stealthy backdoor. I recommend as a hardening measure not to use pickle. For example you can replace pickle with sqlite, json, bson or msgpack, you will need some serializing and deserializing code.
2 eliminate eval as a hardening measure. It doesn't matter if it is exploitable or not, eval is evil and must not be used. Evals are bad for both performance and security. You must remove it. Again, I don't care about your excuses, all the eval
s must be removed.
4 https://github.com/Bitmessage/PyBitmessage/blob/master/src/api.py#L134 Check if timing attack possible here. Also keep in mind that passwords are obsolete and must not be used. Challenge-response protocols are preferred.
@KOLANICH I do have my own ideas about what to do. I'm however looking for someone to do it. If I had more time, I'd do it myself.
Yes, I don't like pickle and abstracted parts of the code to a separate file, now someone needs to refactor it into a class that has its own, safer, storage.
Yes, there are two or three more places with eval, they must be run by someone who already has access to the system so it was a lower priority to fix than the remote execute vulnerability.
class_receiveDataThread.py
is inactive code and should be removed, the line you reference in protocol.py
is also inactive, I just had it there for backwards compatibility as I was changing the code around it. In 0.6.2, PyBitmessage had an random ID that persisted over runtime, to identify if it's connected to itself. Now the ID is unique for each connection.
5 in https://github.com/Bitmessage/PyBitmessage/blob/master/src/bitmsghash/bitmsghash.cpp macrodefinitions are used. It's just dirty and can obfuscate the things going on in code. Macrodefinitions must be avoided, modern C++ must be used instead.
6 https://github.com/Bitmessage/PyBitmessage/blob/master/src/messagetypes/__init__.py#L18
Again, you mustn't import based on remote data. Though it is unexploitable because of check, you must remember that such approach must not be used anywhere. Just prepopulate the dict and select from it. The checking of presense of an item in a list is bad for performance.
7 https://github.com/Bitmessage/PyBitmessage/blob/master/src/class_addressGenerator.py#L110
Why do you use ripemd?
@KOLANICH Are you going to apply?
I'm not.
In light of the recent vulnerability I am looking for experts to audit the code, improve its security and write configuration for security platforms like firejail, apparmor and SElinux.
Applicants please post here in this thread. If you don't want to post publicly, you can contact me privately and we'll discuss how to best apply. An application should contain:
what is your motivation for the application a list of verifiable references of doing similar work (e.g. employer or an open source project) if the auditing wasn't done with python, verifiable references to experience with python a rough proposal for how you would proceed, with an ordered list of tasks (or just sorted into categories like short-term/medium-term/long-term) if you want, you can post publicly how much €€$$ you want, if you don't, I can discuss it privately
no place for newbies, I presume.
Details on reddit: https://www.reddit.com/r/bitmessage/comments/7xrb80/bitmessage_project_looking_for_auditors_andor/