Re: Refactoring base64 encoding and decoding into a safer interface - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Refactoring base64 encoding and decoding into a safer interface
Date
Msg-id 20190702054108.GG1388@paquier.xyz
Whole thread Raw
In response to Re: Refactoring base64 encoding and decoding into a safer interface  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Refactoring base64 encoding and decoding into a safer interface  (Daniel Gustafsson <daniel@yesql.se>)
List pgsql-hackers
On Mon, Jul 01, 2019 at 11:11:43PM +0200, Daniel Gustafsson wrote:
> 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.

Thanks for the review!

> 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

Fixed.

> 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 return value believes it has
> an encoded string?

Hmm.  Good point.  I have not thought of that, and your suggestion
makes sense.

Another question is if we'd want to actually use explicit_bzero()
here, but that could be a discussion on this other thread, except if
the patch discussed there is merged first:
https://www.postgresql.org/message-id/42d26bde-5d5b-c90d-87ae-6cab875f73be@2ndquadrant.com

Attached is an updated patch.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Julien Rouhaud
Date:
Subject: Re: cleanup & refactoring on reindexdb.c
Next
From: Paul Guo
Date:
Subject: Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)