我有一个注册系统,但我真的不知道它是否安全,因为我不太了解安全,我害怕损害我的客户的数据,我相信我创建的方法对安全是有益的,但我仍然怀疑它是否安全
我使用封装创建了几个变量,然后使用它们,对吗?还是应该在方法中创建它们?
正则表达式是否足以防止JavaScript攻击?
我的代码中是否有漏洞或错误?
我们是否可以认为,我已经为我的客户的安全创造了有效的方法?
conn = new PDO("mysql:host=$host;dbname=$dbname", $user, $pass);
$this->conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
return $this->conn;
}catch(PDOException $e) {
throw new Exception($e->getMessage());
}
}
public function setEmail($e) {
$this->email = $e;
}
public function getEmail() {
return $this->email;
}
public function setName($n) {
$this->name = $n;
}
public function getName() {
return $this->name;
}
public function setPassword($password, $confirm) {
if ($password === $confirm) {
$this->password = password_hash($password, PASSWORD_BCRYPT);
} else {
// Handle input error here
echo "Password does not match!";
}
}
public function getPasswordHash() {
return $this->password;
}
// public function CheckHashes() {
// echo $this->password;
// }
public function CheckName(){
$this->check_name = $this->conn->prepare("SELECT name FROM users WHERE name = :name");
$this->check_name->execute(array(
":name" => $this->getName()
));
$this->line = $this->check_name->rowCount();
if($this->line != 0){
$_SESSION['msg'] = 'Name already exists!';
header("Location: signup.php");
exit();
}
}
public function CheckEmail(){
$checkmail = $this->conn->prepare("SELECT email FROM users WHERE email = :email");
$checkmail->execute(array(
":email" => $this->getEmail()
));
$theline = $checkmail->rowCount();
if($theline != 0){
$_SESSION['msg'] = 'Email already exists!';
header("Location: signup.php");
exit();
}
}
public function CheckFields(){
if(empty($this->password) || empty($this->name) || empty($this->email)){
$_SESSION['msg'] = 'Please, fill out all the fields!';
header("Location: signup.php");
exit();
}
}
public function ValidateNameLength(){
if(strlen($this->getName()) < 10){
$_SESSION['msg'] = 'Can only use 10 or more characters as name!';
header("Location: signup.php");
exit();
}
if(strlen($this->getName()) > 25){
$_SESSION['msg'] = 'Can only use a maximum of 25 characters as name!';
header("Location: signup.php");
exit();
}
}
public function ValidateEmail(){
if(!filter_var($this->getEmail(), FILTER_VALIDATE_EMAIL)){
$_SESSION['msg'] = 'Incorrect email!';
header("Location: signup.php");
exit();
}
}
public function Insert() {
if (preg_match("/^[A-Za-z0-9\s]*$/", $this->getName()) ) {
$this->sql = "INSERT INTO users (name, email, pass, ip_address, http_client_ip, http_x_forwarded_for, http_user_agent) VALUES(:name, :email, :pass, :ip_address, :http_client_ip, :http_x_forwarded_for, :http_user_agent)";
$this->result = $this->conn->prepare($this->sql);
$this->remote_addr = $_SERVER['REMOTE_ADDR'];
$this->http_client_ip = $_SERVER['HTTP_CLIENT_IP'];
$this->http_x_forwarded_for = $_SERVER['HTTP_X_FORWARDED_FOR'];
$this->http_user_agent = $_SERVER['HTTP_USER_AGENT'];
$this->result->execute(array(
":name" => $this->getName(),
":email" => $this->getEmail(),
":pass" => $this->getPasswordHash(),
":ip_address" => $this->remote_addr,
":http_client_ip" => $this->http_client_ip,
":http_x_forwarded_for" => $this->http_x_forwarded_for,
":http_user_agent" => $this->http_user_agent
));
if($this->result == true){
$_SESSION['msg'] = 'Registered successfully!';
header("Location: signin.php");
exit();
}
}
else {
$_SESSION['msg'] = 'You can only use letters and numbers as name!';
header("Location: signup.php");
exit();
}
}
}
$obj = new SignUp('localhost', 'dbname', 'user', 'password');
$obj->setEmail('email@gmail.com');
$obj->setName('Anne');
$obj->setPassword('password', 'password');
$obj->CheckName();
$obj->CheckEmail();
$obj->CheckFields();
$obj->ValidateNameLength();
$obj->ValidateEmail();
$obj->Insert();
// $obj->getEmail();
// echo $obj->CheckHashes();
// echo "\n";发布于 2020-11-09 11:30:54
你主要担心的是保安。在您的代码中,我没有看到任何明显的安全问题。要判断您的代码是否真的安全,我需要查看整个系统,而您只提供了其中的一部分。这片看起来很安全,这就是我所能说的。
您的代码有一些实际的样式问题。在我看来你是新来的。我会处理从大到小的问题。
Code重复
有相当多的代码重复。两个明显的例子是:
1:设置错误消息,跳转到signup.php和exit()。可能有这样一种方法:
private function returnWithError($errorMessage)
{
$_SESSION['error'] = $errorMessage;
header('Location: ' . $this->signUpPage);
exit();
} 我将在错误消息中省略HTML标记。HTML应该由将错误输出给用户的代码添加。这更有意义,因为格式化错误的是输出代码的责任。我会再谈这件事的。
2:方法CheckName()和CheckEmail()非常相似。他们都可以使用这样的方法:
private function existingColumnValue($columnName, $value)
{
$statement = $this->database->prepare("SELECT userId FROM users WHERE $columnName = :value");
$statement->execute([':value' => $value]);
return $statement->rowCount() > 0;
}此方法只检查列中是否存在值。它可以用于users表中的所有列。它不是重定向和退出,而是返回一个布尔值。现在您可以将CheckName()和CheckEmail()重写为:
public function existingName($name)
{
return existingColumnValue('name', $name);
}
public function existingEmail($email)
{
return existingColumnValue('email', $email);
}责任
现在,让我们深入研究这个类要做的事情。主要关注的是:是否接受新用户进入数据库。这是它的责任,同样也是如此。因此,设置数据库连接、重定向或格式化输出不应该是该类的一部分。理想情况下,页面底部的代码应该如下所示:
$database = new PDO(.....);
$users = new Users($database);
if ($users->isValidNewUser($name, $email, $password)) {
$userId = $users->addUser($name, $email, $password);
}
else {
$_SESSION['error'] = $users->getLastError();
header('Location: signup.php');
}我根据它与之交互的数据库表命名了这个类。数据库句柄现在在这个类之外创建。这完全取决于你。我使用该类的两种方法:isValidNewUser()和addUser()。这还指向该类的未来扩展,包括getUser()和removeUser()等方法。所以Users类现在所做的就是与数据库交互。当然,您想知道isValidNewUser()和addUser()是什么样子的:
private function setError($errorMessage)
{
$this->errorMessage = $errorMessage;
return FALSE;
}
public function getLastError()
{
return $this->errorMessage;
}
public function isValidNewUser($name, $email, $password)
{
if (empty($name)) return setError('Please supply a name');
if (empty($email)) return setError('Please supply an email address');
if (empty($password)) return setError('Please supply a password');
if (!this->isValidName($name)) return setError('Invalid name. The name should be between 10 and 25 characters. You can only use letters and numbers.');
if ($this->existingName($name)) return setError('This name is already taken, please choose another');
if ($this->existingEmail($email)) return setError('This email address is already registered');
return TRUE;
}
public function addUser($name, $email, $password)
{
$statement = $this->database->prepare("INSERT INTO users (name, email, pass, ip_address, http_client_ip, http_x_forwarded_for, http_user_agent)
VALUES(:name, :email, :password_hash, :ip_address, :http_client_ip, :http_x_forwarded_for, :http_user_agent)");
$statement->execute([':name' => $name,
':email' => $email,
':password_hash' => password_hash($password, PASSWORD_BCRYPT),
':ip_address' => $_SERVER['REMOTE_ADDR'],
':http_client_ip' => $_SERVER['HTTP_CLIENT_IP'],
':http_x_forwarded_for' => $_SERVER['HTTP_X_FORWARDED_FOR'],
':http_user_agent' => $_SERVER['HTTP_USER_AGENT']]);
return $this->database->lastInsertID();
}通过使用两种方法,如果愿意,现在可以跳过验证。如果您想从其他地方导入用户,并且您知道列表是无错误的,这可能很有用。但是,即使使用验证,它也不会重定向,它只设置错误消息。我还没有写出isValidName()方法,因为这个代码示例已经很长了。
<#>Minor其他问题
$this->conn返回__construct()。构造函数总是返回它构造的类,因此其他东西的返回将无法工作。$n,将email参数缩写为$e。为什么?$name和$email不是更好吗?$this->password存储密码哈希。这样做并不少见,但令人困惑。$this->password里有什么?是密码还是散列?为什么不叫它$this->passwordHash呢?$this->check_name中的CheckName()。更多的是,在CheckEmail(),你不会那么做。Check并不能很好地描述该方法的功能。array()替换为[]。我更喜欢它,但是array()也是这样做的。有一个问题我还没有解决:通过将用户数据的验证与insert例程分开,我们就会面临这样的风险:这将无法正常工作。这是不太可能的,但假设两个不同的人,同名,完全同时注册。两种方法都可以根据数据库检查它们的名称,但都找不到,然后都插入。这些不太可能发生的事件往往发生在非常繁忙的地点。这些类型的错误很难调试。
解决这个问题有几种方法。一个经典的方法是使用转换。另一种方法是使用条件插入查询:“如果这个名称不存在,那么插入这个数据。”
不管怎样,我觉得你应该意识到这个小问题。
https://codereview.stackexchange.com/questions/251835
复制相似问题