rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
194 stars 55 forks source link

Make consistent the use of `malloc()` `calloc()` #1277

Closed ronaldtse closed 1 year ago

ronaldtse commented 4 years ago

There are multiple locations where the pair malloc and calloc are used inconsistently. We should either make them consistent or get rid of them.

antonsviridenko commented 1 year ago

What exactly is not ok with malloc()/calloc() use? malloc() is supposed to return uninitialized memory (to save time on zeroing it on each allocation), calloc() returns zero-initialized memory.

ronaldtse commented 1 year ago

I don't remember the exact context when this issue was created (3 years ago). I think this issue was meant to either describe why we use malloc/calloc separately , or about using C++ constructors. @ni4 do you remember this?

ni4 commented 1 year ago

@ronaldtse @antonsviridenko It was quoted from the Cure53 report, and probably more relevant to make sure that malloc doesn't return NULL on all codepathes. Report is opened to public and available here: https://github.com/mozilla/MOSS-Directory/blob/master/SOS_Fund_Audits/thunderbird/RNP-01-report.pdf

antonsviridenko commented 1 year ago

Original quote is

The C/C++ part seems well written but not completely free from inconsistencies, for
example when it comes to the use of malloc()/calloc().

I think it suggests to use C++ way of allocating memory whenever possible, but does not mean there are some security issues in calling these functions.

ni4 commented 1 year ago

@antonsviridenko It is also about the making sure that if allocation fails and NULL is returned then it is safely handled afterwards.

antonsviridenko commented 1 year ago

I've checked the latest master branch https://github.com/rnpgp/rnp/commit/b693a358d8e0b3a4544659f815713188ac96248d

Return values of malloc(), calloc(), realloc() function calls are checked for NULL everywhere except two places in the test code, which should be fixed in #2044

ni4 commented 1 year ago

Closing this as #2044 is already merged.