1

Thank you in advance for reading.

I have an email function that partially works: it does send an email, with the correct message, subject, and headers, when I hard code in the email address. The SQL insertion above it works fine, so I know it is saving all the variables correctly to the database.

It does not send through the $email, $first_names, or $pass variables, however. They come up as blanks in the message and $email does not work in the mail($email, etc) function. Right now, mail() only works when I hard code in an email.

The code, from the PHP file:

$email = $_POST['email'];
$first_names = str_replace("'","",$_POST['fname']);
$last_names = str_replace("'","",$_POST['lname']);
$password = $_POST['password'];
$user_type = 2;
$status = 1;
$last_login = '0000-00-00 00:00:00';
$sql_insert = "INSERT INTO `cb_sub_users` (`id`, `email`, `password`, `firstName`, `lastName`, `user_type`, `super_user`, `last_login`, `status`) VALUES (NULL, '$email', '$password', '$first_names', '$last_names', '$user_type', '$super_user_id', '$last_login', '1');";
$check_user = mysql_num_rows(mysql_query("SELECT * FROM `cb_sub_users` WHERE email='".$email."'"));

$message = 'Hi, ' + $first_names + '! Your team has invited you to join Our Site. Your username is ' . $email . ' and your password is ' . $password . '. Log in at blank for analytics you will actually use. If you have any questions, get in touch at help@help.com.';
$subject = 'Welcome to Our Site';
$headers = array("From: help@help.com",
"Reply-To: help@help.com",
"X-Mailer: PHP/" . PHP_VERSION );
$headers = implode("\r\n", $headers);
$body = "From: $name\n E-Mail: $email\n Message:\n $message";

mail('myhardcoded@email.com', $subject, $message, $headers);
ini_set("mail.log", "/tmp/mail.log");
ini_set("mail.add_x_header", TRUE);

Thanks so much! Any help is much appreciated.

km13oj
  • 54
  • 5
  • `$first_names` OK. `$last_names`. OK. But, from where `$name` coming ? – Nana Partykar Nov 13 '15 at 17:01
  • And, what you are going to do with `$check_user` ? You have not used any where in your code. – Nana Partykar Nov 13 '15 at 17:03
  • Mysql Functions are deprecated in PHP 5.5.0, and it was removed in PHP 7.0.0. Instead, the MySQLi or PDO_MySQL extension should be used. What is `$check_user` doing and are you checking it for a value? – Twisty Nov 13 '15 at 17:06
  • 1
    (1) You set `$check_user` and `$body` but don't use them. (2) You are using `+` in the line `$message = 'Hi, ' + $first_names + '...`. You must use `.`, not `+`, to concatenate strings in PHP. (3) [Don't use `mysql_*`](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php?rq=1); the `mysql_*` functions are outdated, [deprecated](http://us3.php.net/manual/en/intro.mysql.php), and insecure. Use [`MySQLi`](http://us3.php.net/manual/en/book.mysqli.php) or [`PDO`](http://us3.php.net/manual/en/intro.pdo.php) instead. – elixenide Nov 13 '15 at 17:07
  • 1
    (4) You're wide open to [SQL injection](https://www.owasp.org/index.php/SQL_Injection). This code *will* get you hacked. – elixenide Nov 13 '15 at 17:07
  • Thanks for pointing out the vulnerability -- we've fixed it now. Mail still not working, though. Oi vey – km13oj Nov 13 '15 at 18:24
  • @EdCottrell not necessarily. It could be on a site that isn't open to hackers. – Darth Egregious Nov 13 '15 at 18:55
  • @Fuser97381 Defining "hackers" is a fundamental part of most security problems. Disgruntled employees are prime candidates. Anything that provides unintended access to a system is bad, no matter who the user is. – elixenide Nov 13 '15 at 18:58
  • @EdCottrell couldn't agree more! I was just trying to say that if it's locked down to only allow people who already have enough access to eff up other things (i.e. locked behind some other security) then this script doesn't need that protection. Why wear a jock strap if you're already wearing plate male armor? – Darth Egregious Nov 13 '15 at 19:17
  • @Fuser97381 It's impossible to argue with that last statement. :) – elixenide Nov 13 '15 at 19:18

1 Answers1

1

Your code is vulnerable to SQL Injection and is using deprecated MySQL Functions. Here are some things you can do to improve your code:

<?php
$email = isset($_POST['email']?$_POST['email']:"";
$first_name = isset($_POST['fname'])?str_replace("'","",$_POST['fname']):"";
$last_name = isset($_POST['lname'])?str_replace("'","",$_POST['lname']):"";
$password = isset($_POST['password'])?$_POST['password']:"";
if(empty($email)){
    echo "Email is empty.";
    exit();
}
$name = "$first_name $last_name";

$message = "Hi, $first_name! Your team has invited you to join Our Site. Your username is $email and your password is $password. Log in at http://www.help.com/login for social media analytics you will actually use. If you have any questions, get in touch at help@help.com.\r\n";
$subject = 'Welcome to Help';
$headers = array('From: "Help" <help@help.com>',
    "Reply-To: help@help.com",
    "X-Mailer: PHP/" . PHP_VERSION
);
$headers = implode("\r\n", $headers);
$body = "From: $name\n E-Mail: $email\n Message:\n $message";

if(!ini_set("mail.log", "/tmp/mail.log")){
    echo "<p>Mail Log not set.</p>";
}
if(!ini_set("mail.add_x_header", TRUE)){
    echo "<p>X-Headers not enabled.</p>";
}
$result = mail($email, $subject, $message, $headers);
if(!$result){
    echo "<p>Message Failed to Send.</p>";
)
?>

Since it's not clear how you're using your SQL, I just glossed over it since you're asking about mail() here.

Twisty
  • 30,304
  • 2
  • 26
  • 45