As the comments have said, writing a login systems securely is really hard. I would strongly recommend using an existing framework like one of the ones KIKO Software has put in his comment. With that disclaimer out of the way, let's have a look at the code.
<?php
session_start();
if(isset($_SESSION["userid"]))
{
header("location:index.php");
}
Any time you use a redirect, it should be immediately followed by an exit(). If you don't do this the rest of the code will still be executed, and if the user's browser ignores the Location header they can still view the rest of the page. Not so bad here, but can lead to nasty access control issues. HTTP headers are normally capitalised with a space after the colon like Location: /index.php but I don't think there any (common) browser that would complain about this.
if(isset($_POST["login"]))
{
if(!empty($_POST["member_name"]) && !empty($_POST["member_password"]))
A user could submit an array here for the username or password, rather than a string, which you probably don't want, as some PHP functions do strange things with arrays. If you only want a string, you should check for it here.
{
$name = mysqli_real_escape_string($conn, $_POST["member_name"]);
$password = mysqli_real_escape_string($conn, $_POST["member_password"]);
The mysql_real_escape_string() function has some issues with certain character sets, so you need to be careful with it. But since you're using prepared statements, you don't need to escape these, and in fact if you escape them and they have certain characters like single quotes, you'll end up with the wrong values.
$stmt_check = $conn->prepare("SELECT * FROM userz WHERE email =? AND pass = ? AND active ='1'");
Is active a string? I'd expect that to be an integer (or even boolean), but you have single quotes around the 1, so it's being treated as a string by MySQL.
Doing SELECT * is normally a bad sign - you should only select the columns you actually need (which doesn't look like any in this case, you could probably get away with a count).
As mentioned in the comments, storing plaintext passwords is a really bad idea. You should be using a secure hashing function like bcrypt/scrypt/PBKDF2 to store passwords.
$stmt_check->bind_param("ss", $name, $password);
$stmt_check->execute();
$stmt_check->store_result();
$numberofrows = $stmt_check->num_rows;
if(($numberofrows) > 0){
Could the number of rows returned ever by more than 1? That would mean that you have duplicate users in your database, which would be a sign something has gone badly wrong. Unless there's something very strange in the database, it'd be better to check that exactly 1 row has been returned.
setcookie ("member_login",$name,time()+ (10 * 365 * 24 * 60 * 60));
setcookie ("member_password",$password,time()+ (10 * 365 * 24 * 60 * 60));
Storing the plaintext password (or any password) in a cookie is a very bad idea, especially when it's going to be stored for a ten years. Any cookies you're creating should also be marked as HttpOnly, Secure (assuming you're using SSL, which you really should be), and optionally SameSite.
$_SESSION["userid"] = $name;
It's good to be consistent internally with your variable names. So far the same information has been called member_name, name, email and userid
header("location:index.php");
See comments on previous header() usage.
} else {
$message = "Invalid Login OR Email not Verified";
}
$stmt_check->close();
This could have been closed earlier, immediately after you get the number of rows.
As KIKO Software mentioned in his first comment, there are also some other security features that are lacking:
- Account lockout
- CAPTCHA or other brute-force protection
- Session hijacking
Writing authentication code is really, really difficult, and if you solve all of these issues, there are still all kinds of other complexities like timing attacks, user enumeration, etc. You're much better off letting someone else deal with that pain.
