Re: Path Traversal Vulnerability in pg_dump Directory Format

> On 4 Jul 2026, at 15:33, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> On Fri, Jul 3, 2026 at 10:53 PM Imran Zaheer <imran(dot)zhir(at)gmail(dot)com> wrote:

>> + strstr(relativeFilename, "..") != NULL ||
>>
>> This will also reject a valid unix filename i.e. "blob..1.toc" which
>> are unrelated to path traversal. Should we care about such file names
>> here?
>>
> I think instead of strstr we can check direct string "." and ".." as
> changed in my patch..

This doesn't seem to accomplish the same thing though? The former version
checks for ".." embedded in the filename to accomplish traversal, and this
version checks for the directory specifications "." and "..". A filename with
a path traversal would be some version of "../<filename>" to be effective?

Isn't what you want to do more like the below (untested and ugly) sketch which
hunts for DIR_SEP..DIR_SEP or ^..DIR_SEP?

char *p = strstr(relativeFilename, "..");

if (((p > relativeFilename && IS_DIR_SEP(*(p - 1))) || p == relativeFilename)
&& IS_DIR_SEP(*(p + 2)))
pg_fatal(..)

Introducing a pg_fatal on first_dir_separator also triggers foo/bar for path
traversal which would be a false positive.

>>> blob_16388.dat -> ../../../../../../../etc/passwd
>>>
>>> Probably it could be worthy to add the symlink check with lstat() ?
>
> Yeah that makes sense. I have fixed that.

+1 for checking that the resulting pathspec doesn't land on a symlink

+ /*
+ * Per-entry filenames come from the (untrusted) toc.dat / blobs_*.toc.
+ * Refuse empty names, '.' or '..', or anything containing directory
+ * separators.
+ */
+ if (relativeFilename[0] == '\0' ||
+ strcmp(relativeFilename, ".") == 0 ||
+ strcmp(relativeFilename, "..") == 0 ||
+ first_dir_separator(relativeFilename) != NULL)
+ pg_fatal("invalid archive: entry filename \"%s\" is not a plain file name",
+ relativeFilename);

This comment is not entirely accurate, setFilePath is also used by callers who
aren't supplying untrusted input. The error should also not mix "filename" and
"file name" as well as make assumptions that all uses are for an archive.

It would also be good to add a testcase with a malicious TOC to ensure that it
errors out.

--
Daniel Gustafsson