> On 23 Jun 2019, at 15:25, Michael Paquier <mich...@paquier.xyz> wrote:
> Attached is a refactoring patch for those interfaces, which introduces > a set of overflow checks so as we cannot repeat errors of the past. I’ve done a review of this submission. The patch applies cleanly, and passes make check, ssl/scram tests etc. There is enough documentation I very much agree that functions operating on a buffer like this should have the size of the buffer in order to safeguard against overflow, so +1 on the general concept. > Any thoughts? A few small comments: In src/common/scram-common.c there are a few instances like this. Shouldn’t we also free the result buffer in these cases? +#ifdef FRONTEND + return NULL; +#else In the below passage, we leave the input buffer with a non-complete encoded string. Should we memset the buffer to zero to avoid the risk that code which fails to check the returnvalue believes it has an encoded string? + /* + * Leave if there is an overflow in the area allocated for + * the encoded string. + */ + if ((p - dst + 4) > dstlen) + return -1; cheers ./daniel
