Re: Enhance security permissions

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: Bryan Green <dbryan(dot)green(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Enhance security permissions
Date: 2025-11-04 13:17:17
Message-ID: CAEudQAqDAp-8JKGEXxf4XPhPm3O7vWtXEsR9R32q68Yjn7szhw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi.

Em ter., 4 de nov. de 2025 às 09:44, Bryan Green <dbryan(dot)green(at)gmail(dot)com>
escreveu:

> On 11/4/2025 6:20 AM, Ranier Vilela wrote:
> > Hi.
> >
> > I noticed this while checking the source (src/interfaces/libpq/fe-
> > connect.c).
> > It seems that S_IRWXU permission is harmful too.
> >
> > In accord with [1] and [2] this should also be checked.
> > Also, all other places in the source, S_IRWXU are checked.
> >
> > So, I propose adding this check to enhance the security.
> >
> > Maybe the error messages, do they need improvement as well?
> >
> > patchs attached.
> >
> > best regards,
> > Ranier Vilela
> >
> > [1] https://docs.aws.amazon.com/codeguru/detector-library/cpp/loose-
> > file-permissions/ <https://docs.aws.amazon.com/codeguru/detector-
> > library/cpp/loose-file-permissions/>
> > [2] https://www.exploit-db.com/exploits/33145 <https://www.exploit-
> > db.com/exploits/33145>
> I just took a glance an you
> enhance-security-file-permissions-be-secure-common.patch file...
>
> I may be misunderstanding either your intent or what this code actually
> does, but it seems to me that the check rejects files if any of the
> tested bits are set.

Correct.

> Doesn't adding S_IRWXU means rejecting files with
> any owner permissions, including S_IRUSR (owner read).

S_IRWXU on stat is "Mask for file owner permissions".

> That would reject
> mode 0600, which is the documented and required permission for SSL key
> files.
>
I think no.

>
> Mode 0000 would be the only thing that passes this check and we can't
> read that.
>
> I believe your [1] reference is about overly permissive roles in
> creating files. We are validating existing ones.
>
Sorry, I think that [1] has wrong examples of this.

[2] has a more correct example.

We are validating files existing, created by others.
S_IRWXU file mode indicating readable, writable and executable by owner.

I think if the file is executable by the owner, He should be rejected,
shouldn't he?

best regards,
Ranier Vilela

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2025-11-04 13:26:00 Re: Make PGOAUTHCAFILE in libpq-oauth work out of debug mode
Previous Message Alexander Kukushkin 2025-11-04 13:01:45 Re: Issue with logical replication slot during switchover