patternphpMinor
Sanitising contact form input for PHPMailer
Viewed 0 times
contactinputforformsanitisingphpmailer
Problem
I would like some insight on this function for sanitising user submitted data into a contact form. No HTML content or anything, just plain text.
Are there any vulnerabilities here that I'm missing? Am I ok to use the flag
I'm looking to create a all-round function that will be safe to use site-wide.
Note: I'm using this with PHPMailer not
Site-wide functions (sample)...
Feel free to point out any other problems here.
```
// Use instead of error_log to add some useful information to each log
function error_report($err) {
global $user;
error_log('log: ' . $err . ' | User: ' . $user->username . ' | User IP: ' . IP_ADDRESS);
}
// Find & test site / global email settings
function Email_Settings($default = false) {
global $global,$setting;
// Run once & save to $setting StdClass()
if($default == false && !empty($setting->email_settings)) return $setting->email_settings;
else {
// Check for site email settings
if($default == false && !empty($setting->smtp_host) && !empty($setting->smtp_username) && !empty($setting->smtp_password) && !empty($setting->smtp_key)) {
if($smtp_pass = smtp_pass($setting->smtp_password,$setting->smtp_key)) {
if(!empty($smtp_pass)) {
$e = new StdClass();
$e->host = $setting->smtp_host;
$e->username = $setting->smtp_username;
$e->password = $smtp_pass;
$e->receiver = !empty($setting->smtp_receiver) ? $setting->smtp_receiver : $setting->smtp_username;
$e->replyto = $setting->smtp_replyto;
Are there any vulnerabilities here that I'm missing? Am I ok to use the flag
FILTER_FLAG_NO_ENCODE_QUOTES on the subject line? I'm not sure if quotes are safe to submit.I'm looking to create a all-round function that will be safe to use site-wide.
Note: I'm using this with PHPMailer not
mail().// Sanitise headers
function Sanitise_Mail($h) {
$h = filter_var($h,FILTER_SANITIZE_STRING,FILTER_FLAG_NO_ENCODE_QUOTES);
// Taken from PEAR Mail
$h = preg_replace('=((||0x0A/%0A|0x0D/%0D|\\n|\\r)\S).*=i','',$h);
return $h;
}Site-wide functions (sample)...
Feel free to point out any other problems here.
```
// Use instead of error_log to add some useful information to each log
function error_report($err) {
global $user;
error_log('log: ' . $err . ' | User: ' . $user->username . ' | User IP: ' . IP_ADDRESS);
}
// Find & test site / global email settings
function Email_Settings($default = false) {
global $global,$setting;
// Run once & save to $setting StdClass()
if($default == false && !empty($setting->email_settings)) return $setting->email_settings;
else {
// Check for site email settings
if($default == false && !empty($setting->smtp_host) && !empty($setting->smtp_username) && !empty($setting->smtp_password) && !empty($setting->smtp_key)) {
if($smtp_pass = smtp_pass($setting->smtp_password,$setting->smtp_key)) {
if(!empty($smtp_pass)) {
$e = new StdClass();
$e->host = $setting->smtp_host;
$e->username = $setting->smtp_username;
$e->password = $smtp_pass;
$e->receiver = !empty($setting->smtp_receiver) ? $setting->smtp_receiver : $setting->smtp_username;
$e->replyto = $setting->smtp_replyto;
Solution
else if(function_exists('Sanitise_Mail')) {If this condition is false, none of the inner scope executes, right? You could reduce nesting by getting it out of the way first.
Note that your code silently does nothing (no error, no success) if that condition fails: are you missing an
else case? If there's no reason for an else case, then there's no reason to verify whether the function exists.This could be a fun glitch -- for some values of "fun":
$result['error'][] = @$lang->module['invlaidemail'];That's probably intended to be
invalidemail.Code Snippets
else if(function_exists('Sanitise_Mail')) {$result['error'][] = @$lang->module['invlaidemail'];Context
StackExchange Code Review Q#104482, answer score: 5
Revisions (0)
No revisions yet.