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. Doesn't adding S_IRWXU means rejecting files with
any owner permissions, including S_IRUSR (owner read). That would reject
mode 0600, which is the documented and required permission for SSL key
files.
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.
Please correct my understanding as needed.
Thanks,
Bryan
