Hi.

Em ter., 4 de nov. de 2025 às 09:44, Bryan Green <[email protected]>
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

Reply via email to