search

otrv4 / pidgin-otrng

Fork of https://bugs.otr.im/plugins/pidgin-otr. This is a mirror of https://bugs.otr.im/otrv4/pidgin-otrng
GNU General Public License v2.0
16 stars 5 forks source link

Ensure that we check the return value always #96

Open claucece opened 5 years ago

claucece commented 5 years ago

Every malloc, g_malloc, etc.

defreez commented 5 years ago

Hi @claucece

First, thanks for all of your work on OTRv4. It is amazing what you have accomplished.

I am interested in helping with this issue.

I am a PhD student at the University of California, Davis in software engineering (not cryptography). I have been working on a static analysis tool that infers the error values a function may return. The tool also reports callsites where a function may return an error value if a runtime error occurred, but that value is not checked before use.

One of the projects I chose to evaluate the tool on was pidgin-otrng, because of its importance. I configured it to look for return values that result from malloc, g_malloc, calloc, or realloc returning a null pointer. This includes propagation of return values, so the final call may be to another function.

Would you be willing to help with me fix these? I can try to provide fixes, but I will need some help for the cases that are not trivial. Even for not direct calls to malloc or g_malloc, it is not always clear to me what the best recovery action is, as I am not yet familiar with the pidgin-otrng codebase.

The first step would be to simply confirm which cases are actually bugs. As this is a static analysis there is the possibility of false bug reports. I have looked at them, and in my opinion the signal to noise ratio is high enough that they are worth looking at.

Call to get_domain_from_jid at prekey-discovery-jabber.c:253 Call to client_conversation_to_plugin_conversation at plugin-all.c:1366 Call to client_conversation_to_plugin_conversation at plugin-all.c:1382 Call to client_conversation_to_plugin_conversation at plugin-all.c:1395 Call to client_conversation_to_plugin_conversation at plugin-all.c:1530 Call to otrng_gtk_dialog_add_smp_data at gtk-dialog.c:2742 Call to malloc at prekey-plugin-peers.c:174 Call to malloc at prekey-plugin-peers.c:178 Call to malloc at prekey-discovery-jabber.c:256 Call to malloc at prekey-discovery-jabber.c:189 Call to malloc at prekey-discovery-jabber.c:154 Call to malloc at prekey-discovery-jabber.c:122 Call to malloc at prekey-discovery-jabber.c:62 Call to malloc at prekey-discovery-jabber.c:48 Call to malloc at plugin-all.c:1283 Call to malloc at gtk-ui.c:550 Call to malloc at gtk-ui.c:299 Call to malloc at gtk-ui.c:230 Call to malloc at gtk-dialog.c:2723 Call to malloc at gtk-dialog.c:1411 Call to malloc at gtk-dialog.c:2494 Call to malloc at gtk-dialog.c:772 Call to malloc at gtk-dialog.c:2841 Call to otrng_plugin_prekey_domain_for at prekey-plugin-account.c:105 Call to otrng_plugin_prekey_domain_for at prekey-plugin-account.c:183 Call to otrng_plugin_prekey_domain_for at prekey-plugin-shared.c:91 Call to otrng_plugin_prekey_domain_for at plugin-all.c:648 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:1866 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:2647 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:2453 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:1990 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:1319 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:2827 Call to purple_conversation_to_plugin_conversation at gtk-dialog.c:2921 Call to hex_to_bytes at prekey-discovery-jabber.c:60 Call to g_malloc at plugin-all.c:1313 Call to g_malloc at plugin-all.c:1025 Call to g_malloc at plugin-all.c:1030 Call to copy_known_fingerprint_v3 at gtk-ui.c:232 Call to conn_context_to_plugin_conversation at gtk-dialog.c:1882 Call to g_malloc at gtk-dialog.c:2680 Call to g_malloc at gtk-dialog.c:2684 Call to g_malloc at gtk-dialog.c:2689 Call to g_malloc at gtk-dialog.c:2694 Call to otrng_plugin_conversation_copy at gtk-dialog.c:938 Call to create_smp_progress_dialog at gtk-dialog.c:360 Call to otrng_plugin_fingerprint_new at gtk-dialog.c:775 Call to otrng_plugin_fingerprint_new at gtk-dialog.c:781 Call to vrfy_fingerprint_data_new at gtk-dialog.c:818 Call to otrng_plugin_conversation_copy at gtk-dialog.c:1145 Call to vrfy_fingerprint_data_new at gtk-dialog.c:1603

claucece commented 5 years ago

Hi, @defreez !

First, thanks for all of your work on OTRv4. It is amazing what you have accomplished.

Oh, thank you! It is a work of a whole team ;)

I am a PhD student at the University of California, Davis in software engineering (not cryptography). I have been working on a static analysis tool that infers the error values a function may return. The tool also reports callsites where a function may return an error value if a runtime error occurred, but that value is not checked before use.

Oh, it seems very awesome! I'll check it out (the tool).

Would you be willing to help with me fix these? I can try to provide fixes, but I will need some help for the cases that are not trivial. Even for not direct calls to malloc or g_malloc, it is not always clear to me what the best recovery action is, as I am not yet familiar with the pidgin-otrng codebase.

For sure! I will start checking ;)

Thanks so much!