Skip to content

Add blacklisted command line flags for sendline via mail() #12783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add blacklisted command line flags for sendline via mail() #12783

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1d26b7a
Added mail.allow_additional_sendmail_flags php.ini-development
TCGeek Nov 25, 2023
b4a57b9
Added mail.allow_additional_sendmail_flags php.ini-production
TCGeek Nov 25, 2023
440b2ca
Added blacklist functionality mail.c
TCGeek Nov 25, 2023
fae7507
Declared php_mail_disable_flags php_mail.h
TCGeek Nov 25, 2023
ec0cd1c
Added bool for mail.allow_additional_sendmail_flags main.c
TCGeek Nov 25, 2023
7d7eff5
Added globals for allow_additional_sendmail_flags php_globals.h
TCGeek Nov 25, 2023
ef0b0ef
Create allowed test mail_blacklist_allowed.phpt
TCGeek Nov 25, 2023
10b4625
Create disallow test mail_blacklist_disallowed.phpt
TCGeek Nov 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Added blacklist functionality mail.c
  • Loading branch information
@TCGeek
TCGeek authored Nov 25, 2023
commit 440b2ca80a514026063a2def2cc7bd09609cf949
25 changes: 25 additions & 0 deletions ext/standard/mail.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,28 @@ PHPAPI zend_string *php_mail_build_headers(HashTable *headers)
return s.s;
}

/* {{{ php_mail_disable_flags
Remove all dangerous command line flags
from the sendmail shell command
*/
PHPAPI zend_string *php_mail_disable_flags(const char *str)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pass in the str_length as an argument, we know the size and this prevents a useless computation.

Also, this will just truncate the command if it contains a null byte.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why is this exposed as a PHPAPI? can't it be static

{
const char* blacklist[4] = {"-O", "-C", "-X", "-be"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be better to make this configurable as a list of potentially insecure flags changeable using INI. You could keep those as default.

size_t str_length = strlen(str);
char *return_str = emalloc(str_length + 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this allocation really needed?

You can just allocate the zend_string directly, and iterate on the underlying char pointer.

strcpy(return_str, str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use memcpy.

for (int i = 0; i < 4; ++i) {
size_t blacklist_length = strlen(blacklist[i]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really not a fan of this, those strings are constant and computing the lengths at runtime really feels off

char* position = return_str;
while ((position = strstr(position, blacklist[i])) != NULL) {
memset(position, ' ', blacklist_length);
position += blacklist_length;
}
}
zend_string *cmd = zend_string_init(return_str, str_length, 0);
efree(return_str);
return cmd;
}

/* {{{ Send an email message */
PHP_FUNCTION(mail)
Expand Down Expand Up @@ -278,6 +300,9 @@ PHP_FUNCTION(mail)
if (force_extra_parameters) {
extra_cmd = php_escape_shell_cmd(force_extra_parameters);
} else if (extra_cmd) {
if (!PG(allow_additional_sendmail_flags)) {
extra_cmd = php_mail_disable_flags(ZSTR_VAL(extra_cmd));
}
extra_cmd = php_escape_shell_cmd(ZSTR_VAL(extra_cmd));
}

Expand Down