Thread
-
Path Traversal Vulnerability in pg_dump Directory Format
Dilip Kumar <dilipbalaut@gmail.com> — 2026-07-03T06:51:57Z
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. -- Regards, Dilip Kumar Google
-
Re: Path Traversal Vulnerability in pg_dump Directory Format
Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> — 2026-07-03T15:07:17Z
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
-
Re: Path Traversal Vulnerability in pg_dump Directory Format
Imran Zaheer <imran.zhir@gmail.com> — 2026-07-03T17:23:38Z
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: > > > 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 > >
-
Re: Path Traversal Vulnerability in pg_dump Directory Format
Dilip Kumar <dilipbalaut@gmail.com> — 2026-07-04T13:33:25Z
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
-
Re: Path Traversal Vulnerability in pg_dump Directory Format
Daniel Gustafsson <daniel@yesql.se> — 2026-07-06T14:12:42Z
> 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