Attachments are created without user read permission (--w-rwSr--)

[SharkWipf]

• Faveo Version : Git master (v1.10.4), but also applies to some older versions, unsure which.
• PHP version : 7.1.23
• Database Driver & Version : mysqld Ver 5.5.41-MariaDB for Linux on x86_64 (MariaDB Server)
• Server specification : CentOS 7

Description:

As of recent-ish versions, mail attachments are created without user read permissions.
For some reason setgid is also set.
The exact permissions set on attachments are now --w-rwSr--, or 2264.
This causes, among other things, 3rd party backup scripts to fail.

Attachments, on a much older version of Faveo, used to be created with the (presumably inherited) permissions of -rw-r--r--.

Steps To Reproduce:

• Send in a mail ticket with file attachment.
• Check the permissions on the created file in storage/app/attachments/.

Downloaded from

• master-branch

• development-branch

• release-tag

[SharkWipf]

To clarify: This issue does not affect the usability of the Faveo software itself, attachments work fine within Faveo itself, but anything trying to access the files outside of the system, like a backup script, will fail.

[bhanu2217]

Can you please suggest a solution, we have these permission. We had changed permission due to security angle. Someone being able to upload malicious files and execute them.

[SharkWipf]

@bhanu2217 Apologies, I missed your reply somehow.
I've taken a look at the actual code, and it seems this code itself is bugged.
PHP's chmod does not behave as one would expect, and any chmod mode value not starting with a 0 is treated in strange and, from what I could tell, undocumented ways.

Commit 03cdb39 introduced 2 broken chmods in the code, here and here.

Like I said above, any chmod mode value not starting with a 0 is treated in strange ways.

To properly set the permissions to 1204 the syntax would be chmod($root, 01204).
However, I doubt this (--w----r-T) is the intended permission set either, and the intended permissions would probably be chmod($root, 0644), in-line with some other chmods in the code, or 0640, or even 0600 (only user can read/write, no-one else).

I am guessing the reason why this got through might be because passing "bad" permissions to PHP's chmod() might produce different results on different machines.

I've submitted a pull request changing the chmods to 0644, as I think this is what was originally intended and has least risk of breaking other things, however changing this to 0600 might be even better.

[antiphase]

I've taken a look at the actual code, and it seems this code itself is bugged.
PHP's chmod does not behave as one would expect, and any chmod mode value not starting with a 0 is treated in strange and, from what I could tell, undocumented ways.

The leading 0 is standard C notation to denote an octal value, which is what chmod conventionally uses. You should really express these values as 5 digits; a leading 0 then 4 more octal digits to represent the 12 bits of POSIX file permissions; i.e. 00644 in the intended case.

02264 octal is 1204 decimal, which is what you are seeing file permissions set to.

[SharkWipf]

Makes sense.

You should really express these values as 5 digits; a leading 0 then 4 more octal digits to represent the 12 bits of POSIX file permissions; i.e. 00644 in the intended case.

While this sounds sensible, there's no mention of this notation even on PHP.net.
They only add the 5th digit if they explicitly need the 4th bit.

02264 octal is 1204 decimal, which is what you are seeing file permissions set to.

Yeah makes sense.

What I don't understand then is how this code even made it in in the first place, and how the developer ended up at chmod(1204) and decided the result made any sense at all.
I honestly can't think of any scenario in which this permission set would actually make sense, in fact, the permission set conflicts with itself (sticky group bit without group execute does not even work, denoted by the capital S in the permissions string).