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

On Thu, 30 Jan 2025 at 16:47, Srinath Reddy <srinath2133(at)gmail(dot)com> wrote:
>
>
>
> On Wed, Jan 29, 2025 at 9:55 PM Mahendra Singh Thalor <mahi6run(at)gmail(dot)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(at)leadboat(dot)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.

[1] : names with \n\r in dbnames
<https://www.postgresql.org/message-id/4ef51faa-993f-46ea-9e68-7baf736c07b8%40dunslane.net>

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