Re: Unbounded %s in sscanf

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Unbounded %s in sscanf
Date: 2021-07-03 20:18:54
Message-ID: A2B151F5-B32B-4F2C-BA4A-6870856D9BDE@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 28 Jun 2021, at 16:45, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
>> On 28 Jun 2021, at 16:02, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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 Content-Type Size
v3-0003-Fix-bug-in-TOC-file-error-message-printing.patch application/octet-stream 2.2 KB
v3-0002-Fix-sscanf-limit-in-pg_dump.patch application/octet-stream 1.4 KB
v3-0001-Fix-sscanf-limit-in-pg_basebackup.patch application/octet-stream 1.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2021-07-03 21:46:58 Re: Preventing abort() and exit() calls in libpq
Previous Message Jeff Davis 2021-07-03 18:44:20 Re: Synchronous commit behavior during network outage