Horton

Re: Unbounded %s in sscanf

Daniel Gustafsson <daniel@yesql.se>

View thread View on postgr.es
From: Daniel Gustafsson <daniel@yesql.se>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-07-03T20:18:54Z
Lists: pgsql-hackers

Attachments

> 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/

Commits

  1. Fix bug in TOC file error message printing

  2. Fix sscanf limits in pg_basebackup and pg_dump

  3. Fix sscanf limits in pg_dump