Le 09/10/2016 à 11:48, Pavel Stehule a écrit :
> hi
>
> 2016-10-04 9:18 GMT+02:00 Gilles Darold <gilles.dar...@dalibo.com
> <mailto:gilles.dar...@dalibo.com>>:
>
>     Le 03/10/2016 à 23:23, Gilles Darold a écrit :
>     > Le 03/10/2016 à 23:03, Robert Haas a écrit :
>     >> On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold
>     <gil...@darold.net <mailto:gil...@darold.net>> wrote:
>     >>> 4) An other problem is that like this this patch will allow
>     anyone to upload into a
>     >>> column the content of any system file that can be read by
>     postgres system user
>     >>> and then allow non system user to read its content.
>     >> I thought this was a client-side feature, so that it would let a
>     >> client upload any file that the client can read, but not things
>     that
>     >> can only be read by the postgres system user.
>     >>
>     > Yes that's right, sorry for the noise, forget this fourth report.
>     >
>
>     After some more though there is still a security issue here. For a
>     PostgreSQL user who also have login acces to the server, it is
>     possible
>     to read any file that the postgres system user can read, especially a
>     .pgpass or a recovery.conf containing password.
>
>
> here is new update - some mentioned issues are fixed + regress tests
> and docs

Looks very good for me minus the two following points:

1) I think \setfileref must return the same syntax than \set command

    postgres=# \setfileref a testfile.txt
    postgres=# \setfileref
    a = 'testfile.txt'

    postgres=# \setfileref
    ...
    a = ^'testfile.txt'

I think it would be better to prefixed the variable value with the ^ too
like in the \set report even if we know by using this command that
reported variables are file references.


2) You still allow special file to be used, I understand that this is
from the user responsibility but I think it could be a wise precaution. 

    postgres=# \setfileref b /dev/random
    postgres=# insert into test (:b);

Process need to be killed using SIGTERM.

However if this last point is not critical and should be handle by the
user, I think this patch is ready to be reviewed by a committer after
fixing the first point.

Regards,

-- 
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

Reply via email to