Re: Unbounded %s in sscanf - Mailing list pgsql-hackers

From Daniel Gustafsson
Subject Re: Unbounded %s in sscanf
Date
Msg-id A2B151F5-B32B-4F2C-BA4A-6870856D9BDE@yesql.se
Whole thread Raw
In response to Re: Unbounded %s in sscanf  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Unbounded %s in sscanf  (Daniel Gustafsson <daniel@yesql.se>)
List pgsql-hackers
> On 28 Jun 2021, at 16:45, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> On 28 Jun 2021, at 16:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Ugh.  Shouldn't we instead modify the format to read not more than
>> two characters?  Even if this is safe on non-malicious input, it
>> doesn't seem like good style.
>
> No disagreement, I was only basing it on what is in the tree.  I would propose
> that we change the sscanf in _LoadBlobs() too though to eliminate all such
> callsites, even though that one is even safer.  I'll prepare a patch once more
> caffeine has been ingested.

Returning to this, attached is a patchset which amends the two sscanf()
callsites with their respective buffersizes for %s format parsing.  In pg_dump
we need to inject the MAXPGPATH limit with the preprocessor and thus the buffer
needs to be increased by one to account for the terminator (which is yet more
hygiene coding since the fname buffer is now larger than the input buffer).

While in here, I noticed that the fname variable is shadowed in the loop
parsing the blobs TOC which yields a broken error message on parse errors.  The
attached 0003 fixes that.

--
Daniel Gustafsson        https://vmware.com/


Attachment

pgsql-hackers by date:

Previous
From: Jeff Davis
Date:
Subject: Re: Synchronous commit behavior during network outage
Next
From: Noah Misch
Date:
Subject: Re: Preventing abort() and exit() calls in libpq