> On 23 Jun 2019, at 15:25, Michael Paquier <michael@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