Hello Yasuoka-san, YASUOKA Masahiko wrote on Thu, Aug 01, 2019 at 08:42:35PM +0900:
> I noticed the upstream NetBSD recently replaced almost all malloc(3)s > by calloc(3) in libedit. > > https://github.com/NetBSD/src/commit/b91b3c48e0edb116bd797586430cb426b575d717 > > This also fixes the problem. I'll create a diff which does the same > thing. Might i suggest that you first send a diff changing only the one place required to fix the bug asou@ reported? That can easily be reviewed, and i expect we can get it in quickly, and it allows a useful commit message explaining why exactly it is needed. Regarding all the other places in the library: it is true that in many situations, zeroing buffers when allocating them provides additional safety. But there may be exceptions; in some cases, it may hide or rarely even cause bugs. In any case, i hate it when people go "i have no idea whether it is needed and what it does, but let's just change this globally anyway". Even if it's about something as seemingly unconspicious as zeroing buffers. The NetBSD commit message provides no indication that the changed code was inspected for consequences of the change, and as far as i know Christos (admittedly, i only met him two or three times and didn't talk all *that* much to him) i guess it might indeed be his style to commit changes without actually inspecting the code. So i think for committing to OpenBSD, each function changed needs to be inspected and it needs to be confirmed that zeroing each buffer in question does not cause new problems or hide other bugs. That may be somewhat painful work, libedit code is not very audit friendly, but i don't think cutting corners is a good idea. In general, code quality in libedit is somewhat below OpenBSD quality standards, so the time spent auditing it is certainly not wasted but spent at a place needing it. Also, the code quality implies that zeroing some additional buffers likely improves security at least at some of the places - if it is checked properly. Yours, Ingo
