Path Traversal Vulnerability in pg_dump Directory Format

Hello!!

Dilip Kumar <dilipbalaut@gmail.com> writes:

I would like to submit a patch to address a path traversal
vulnerability in pg_dump's directory format mode (-F d). Currently,
filenames listed in directory-format TOC files (toc.dat and
blobs_*.toc) are treated as trusted when reading an archive during a
restore. If an archive entry filename is maliciously modified to
contain path traversal elements (such as ..) or directory separators,
pg_restore can be tricked into reading files outside the intended
backup directory. The attached patch fixes this vulnerability.

I was taking a look into the patch and, yes it works as expected, but I
also manage to get the same result of a path traversal having a with a
symlink as follow:

blob_16388.dat -> ../../../../../../../etc/passwd

Probably it could be worthy to add the symlink check with lstat() ?

Regards,
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com

Hi

+ 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?

Thanks
Imran Zaheer

On Fri, Jul 3, 2026 at 8:07 PM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:

On Fri, Jul 3, 2026 at 10:53 PM Imran Zaheer <imran.zhir@gmail.com> wrote:

Hi

+ 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..

On Fri, Jul 3, 2026 at 8:07 PM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:

Hello!!

Dilip Kumar <dilipbalaut@gmail.com> writes:

I would like to submit a patch to address a path traversal
vulnerability in pg_dump's directory format mode (-F d). Currently,
filenames listed in directory-format TOC files (toc.dat and
blobs_*.toc) are treated as trusted when reading an archive during a
restore. If an archive entry filename is maliciously modified to
contain path traversal elements (such as ..) or directory separators,
pg_restore can be tricked into reading files outside the intended
backup directory. The attached patch fixes this vulnerability.

I was taking a look into the patch and, yes it works as expected, but I
also manage to get the same result of a path traversal having a with a
symlink as follow:

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.

--
Regards,
Dilip Kumar
Google

On 4 Jul 2026, at 15:33, Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Fri, Jul 3, 2026 at 10:53 PM Imran Zaheer <imran.zhir@gmail.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

Hello!

I was taking a look into the patch and, yes it works as expected, but I
also manage to get the same result of a path traversal having a with a
symlink as follow:

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.

I've tested the patch and now the error it's clear:

pg_restore -f testo-bad3.sql testo.dump-traversal/
pg_restore: error: invalid archive: entry file "testo.dump-traversal//blob_123.dat" is a symbolic link

The only thing that it was a bit weird to me, is that the check using
lstat() was made after checking the traversal path, since this check is
less expensive in terms of functions calls, probably it could be
worthy?

Regards!
--
Jonathan Gonzalez V.
EDB
https://www.enterprisedb.com