db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-03-17 20:58:34 +02:00
Code Issues Releases Activity

Fix issues in improved path validation for repo-* commands.

Browse Source 
If the user requested the exact repo path then strSub() would be passed an invalid start value leading to an assertion:

$ pgbackrest --stanza=test repo-ls /var/lib/pgbackrest
ASSERT: [025]: start <= this->pub.size (on dev builds)
ASSERT: [025]: string size must be <= 1073741824 bytes (on prod builds)

Fix this by checking if the requested path exactly equals the repo path and returning an empty relative path in this case.

Another issue was that invalid subpaths were not detected if they started with the repo path. For example, /var/lib/pgbackrestsub would not generate an error if the repo path was /var/lib/pgbackrest. Fix this by explictly checking for a / between the repo path and the subpath. This also requires special handling when the repo path is /.

This is not a live bug since the issues were found in an unreleased feature introduced in 5ae84d5.
This commit is contained in:
David Steele 2022-05-13 09:41:53 -04:00 committed by GitHub
parent 024500782e
commit 19dd015d58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 54 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/xml/release.xml
View File

@ -188,7 +188,12 @@
</release-item>
<release-item>
<github-pull-request id="1697"/>
<commit subject="Improve path validation for repo-* commands.">
<github-pull-request id="1697"/>
</commit>
<commit subject="Fix issues in improved path validation for repo-* commands.">
<github-pull-request id="1751"/>
</commit>
<release-item-contributor-list>
<release-item-contributor id="reid.thompson"/>

35
src/command/repo/common.c
View File

@ -32,21 +32,34 @@ repoPathIsValid(const String *path)
// Validate absolute paths
if (strBeginsWith(path, FSLASH_STR))
{
// Check that the file path begins with the repo path
if (!strBeginsWith(path, cfgOptionStr(cfgOptRepoPath)))
// If the path is exactly equal to the repo path then the relative path is empty
if (strEq(path, cfgOptionStr(cfgOptRepoPath)))
{
THROW_FMT(
ParamInvalidError, "absolute path '%s' is not in base path '%s'", strZ(path),
strZ(cfgOptionDisplay(cfgOptRepoPath)));
MEM_CONTEXT_PRIOR_BEGIN()
{
result = strNew();
}
MEM_CONTEXT_PRIOR_END();
}
// Else check that the file path begins with the repo path
else
{
if (!strEq(cfgOptionStr(cfgOptRepoPath), FSLASH_STR) &&
!strBeginsWith(path, strNewFmt("%s/", strZ(cfgOptionStr(cfgOptRepoPath)))))
{
THROW_FMT(
ParamInvalidError, "absolute path '%s' is not in base path '%s'", strZ(path),
strZ(cfgOptionDisplay(cfgOptRepoPath)));
}
MEM_CONTEXT_PRIOR_BEGIN()
{
// Get the relative part of the file
result = strSub(
path, strEq(cfgOptionStr(cfgOptRepoPath), FSLASH_STR) ? 1 : strSize(cfgOptionStr(cfgOptRepoPath)) + 1);
MEM_CONTEXT_PRIOR_BEGIN()
{
// Get the relative part of the path/file
result = strSub(
path, strEq(cfgOptionStr(cfgOptRepoPath), FSLASH_STR) ? 1 : strSize(cfgOptionStr(cfgOptRepoPath)) + 1);
}
MEM_CONTEXT_PRIOR_END();
}
MEM_CONTEXT_PRIOR_END();
}
else
{

25
test/src/module/command/repoTest.c
View File

@ -96,6 +96,18 @@ testRun(void)
cfgOptionSet(cfgOptFilter, cfgSourceParam, NULL);
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("exact repo path");
StringList *argListTmp = strLstDup(argList);
strLstAddZ(argListTmp, TEST_PATH "/repo");
HRN_CFG_LOAD(cfgCmdRepoLs, argListTmp);
output = bufNew(0);
cfgOptionSet(cfgOptOutput, cfgSourceParam, VARUINT64(CFGOPTVAL_OUTPUT_TEXT));
TEST_RESULT_VOID(storageListRender(ioBufferWriteNew(output)), "empty directory (text)");
TEST_RESULT_STR_Z(strNewBuf(output), "", "check output");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("add path and file");
@ -160,7 +172,7 @@ testRun(void)
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("error on /");
StringList *argListTmp = strLstDup(argList);
argListTmp = strLstDup(argList);
strLstAddZ(argListTmp, "/");
HRN_CFG_LOAD(cfgCmdRepoLs, argListTmp);
@ -168,6 +180,17 @@ testRun(void)
storageListRender(ioBufferWriteNew(output)), ParamInvalidError,
"absolute path '/' is not in base path '" TEST_PATH "/repo'");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("error on path that starts with repo path");
argListTmp = strLstDup(argList);
strLstAddZ(argListTmp, TEST_PATH "/reposub");
HRN_CFG_LOAD(cfgCmdRepoLs, argListTmp);
TEST_ERROR(
storageListRender(ioBufferWriteNew(output)), ParamInvalidError,
"absolute path '" TEST_PATH "/reposub' is not in base path '" TEST_PATH "/repo'");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("error on //");