|From:||Daniel Gustafsson <daniel(at)yesql(dot)se>|
|To:||Michael Paquier <michael(at)paquier(dot)xyz>|
|Cc:||Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>|
|Subject:||Re: Refactoring base64 encoding and decoding into a safer interface|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
> On 23 Jun 2019, at 15:25, Michael Paquier <michael(at)paquier(dot)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
> 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?
+ return NULL;
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;
|Next Message||Peter Geoghegan||2019-07-01 21:21:40||Re: Add parallelism and glibc dependent only options to reindexdb|
|Previous Message||Julien Rouhaud||2019-07-01 20:34:47||Re: Add parallelism and glibc dependent only options to reindexdb|