Ensure that we check the return value always

[claucece]

Every malloc, g_malloc, etc.

[defreez]

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]

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!