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

From Michael Paquier
Subject Refactoring base64 encoding and decoding into a safer interface
Date
Msg-id 20190623132535.GB1628@paquier.xyz
Whole thread Raw
Responses Re: Refactoring base64 encoding and decoding into a safer interface
List pgsql-hackers
Hi all,

After the issues behind CVE-2019-10164, it seems that we can do much
better with the current interface of decoding and encoding functions
for base64 in src/common/.

The root issue is that the callers of pg_b64_decode() and
pg_b64_encode() provide a buffer where the result gets stored which is
allocated using respectively pg_b64_dec_len() and pg_b64_dec_enc()
(those routines overestimate the allocation on purpose) but we don't
allow callers to provide the length of the buffer allocated and hence
those routines lack sanity checks to make sure that what is in input
does not cause an overflow within the result buffer.

One thing I have noticed is that many projects on the net include this
code for their own purpose, and I have suspicions that many other
projects link to the code from Postgres and make use of it.  So that's
rather scary.

Attached is a refactoring patch for those interfaces, which introduces
a set of overflow checks so as we cannot repeat errors of the past.
This adds one argument to pg_b64_decode() and pg_b64_encode() as the
size of the result buffer, and we make use of it in the code to make
sure that an error is reported in case of an overflow.  That's the
status code -1 which is used for other errors for simplicity.  One
thing to note is that the decoding path can already complain on some
errors, basically an incorrectly shaped encoded string, but the
encoding path does not have any errors yet, so we need to make sure
that all the existing callers of pg_b64_encode() complain correctly
with the new interface.

I am adding that to the next CF for v13.

Any thoughts?
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: Index Skip Scan
Next
From: Michael Paquier
Date:
Subject: Re: New vacuum option to do only freezing