Re: Forbid use of LF and CR characters in database and role names

On Wed, Sep 7, 2016 at 2:32 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I think probably a better answer is to reject bad paths earlier, eg have
> initdb error out before doing anything if the proposed -D path contains
> CR/LF.

Yes, that's a bug that we had better address. It is not nice to not
clean up PGDATA in this case.

> Given the collection of security bugs we just fixed, and the
> strong likelihood that user-written scripts contain other instances of
> the same type of problem, I do not think we are doing anybody any favors
> if we sweat bullets to support such paths.

Yep. Database and role names are the two patterns defined by the user
at SQL level that can be reused by command lines, so it still seems to
me that the patch I sent is the correct call..

> And I'm not even convinced
> that we *can* support such paths on Windows; no one was able to find a
> safe shell quoting solution there.

None has been found as far as I know, the problem gets into Windows
core with paths passed to cmd.exe which cannot handle those two
characters correctly.

> And yeah, the docs probably need work.

Patch 0001 is what I have done for the database and role names. Patch
0002 adds a routine in string_utils.c to check if a string given is
valid for shell commands or not. I added a call of that to initdb.c,
which is at least what we'd want to check because it calls directly
appendShellString. Now I am a bit reluctant to spread that
elsewhere... Users would get the message only with initdb if they see
the problem. I have added a note in the page of initdb as well, but
that does not sound enough to me, we may want to add a warning in a
more common plase. Does somebody have an idea where we could add that.

Also, in 0001 I have cleared the docs to refer to newline and carriage
return characters instead of CR and LF.
--
Michael