Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote - Mailing list pgsql-hackers

From Mahendra Singh Thalor
Subject Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
Date
Msg-id CAKYtNAreuQv04Mfy-u=TnOGrp2ofagRPV43XjjnbJb_5JDqC+Q@mail.gmail.com
Whole thread Raw
Responses Re: getting "shell command argument contains a newline or carriage return:" error with pg_dumpall when db name have new line in double quote
List pgsql-hackers
On Thu, 30 Jan 2025 at 16:47, Srinath Reddy <srinath2133@gmail.com> wrote:
>
>
>
> On Wed, Jan 29, 2025 at 9:55 PM Mahendra Singh Thalor <mahi6run@gmail.com> wrote:
>>
>> Hi,
>> While doing some testing with pg_dumpall, I noticed one weird behaviour.
>>
>> While we create the database, we are allowing the database name with a new line (if name is in double quote).
>> For example:
>>>
>>> postgres=# create database "dbstr1;
>>> dbstr 2";
>>> CREATE DATABASE
>>> postgres=#
>>
>> Here, the database name is in 2 lines.
>>
>> With the help of pg_dumpall, I tried to dump but I am getting an error for the new line.
>>
>>> --
>>> -- Database "dbstr1;
>>> dbstr 2" dump
>>> --
>>>
>>> shell command argument contains a newline or carriage return: " dbname='dbstr1;
>>> dbstr 2'"
>>
>>  
>> After this message, we are stopping the dump.
>
>
> I have reproduced and verified the same.The reason is in runPgDump during appendShellString for forming the pg_dump command , in appendShellStringNoError we are considering the string as invalid if it has '\n' and '\r'.
>  
>>
>>
>> I think, if we are allowing new lines in the db name, then we should dump it.
>

In another thread[1], we have some discussions regarding \n\r in dbname and they think that we should fix it.

As per code,
*
 * Append the given string to the shell command being built in the buffer,
 * with shell-style quoting as needed to create exactly one argument.
 *
 * Forbid LF or CR characters, which have scant practical use beyond designing
 * security breaches.  The Windows command shell is unusable as a conduit for
 * arguments containing LF or CR characters.  A future major release should
 * reject those characters in CREATE ROLE and CREATE DATABASE, because use
 * there eventually leads to errors here.

 *
 * appendShellString() simply prints an error and dies if LF or CR appears.
 * appendShellStringNoError() omits those characters from the result, and
 * returns false if there were any.
 */
void
appendShellString(PQExpBuffer buf, const char *str)
{
    if (!appendShellStringNoError(buf, str))
    {
        fprintf(stderr,
                _("shell command argument contains a newline or carriage return: \"%s\"\n"),
                str);
        exit(EXIT_FAILURE);
    }
}

Here, we are mentioning that in future majar releases, we should reject \n\r in CREATE ROLE and CREATE DATABASE.

Above comment was added in 2016.
commit 142c24c23447f212e642a0ffac9af878b93f490d
Author: Noah Misch <noah@leadboat.com>
Date:   Mon Aug 8 10:07:46 2016 -0400

    Reject, in pg_dumpall, names containing CR or LF.
   
    These characters prematurely terminate Windows shell command processing,
    causing the shell to execute a prefix of the intended command.  The
    chief alternative to rejecting these characters was to bypass the
    Windows shell with CreateProcess(), but the ability to use such names
    has little value.  Back-patch to 9.1 (all supported versions).
   
    This change formally revokes support for these characters in database
    names and roles names.  Don't document this; the error message is
    self-explanatory, and too few users would benefit.  A future major
    release may forbid creation of databases and roles so named.  For now,
    check only at known weak points in pg_dumpall.  Future commits will,
    without notice, reject affected names from other frontend programs.
   
    Also extend the restriction to pg_dumpall --dbname=CONNSTR arguments and
    --file arguments.  Unlike the effects on role name arguments and
    database names, this does not reflect a broad policy change.  A
    migration to CreateProcess() could lift these two restrictions.
   
    Reviewed by Peter Eisentraut.
   
    Security: CVE-2016-5424

As per above comments, we can work on a patch which will reject \n\r in roles and database names.

I will work on this.

 
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Joe Conway
Date:
Subject: Re: Support "make check" for PGXS extensions
Next
From: Antonin Houska
Date:
Subject: Re: why there is not VACUUM FULL CONCURRENTLY?