patternphpMinor
One-time secret message application using PHP-encryption library
Viewed 0 times
encryptionapplicationphpmessagetimeoneusingsecretlibrary
Problem
By utilizing an existing PHP encryption library (defuse/php-encryption) and a flat file database library (jamesmoss/flywheel), my application take a secret message from a user, encrypts it using a key derived from a password, and saves it to a flat file. The message can then be decrypted by visiting a unique link and providing the same password. After the message is viewed it is automatically deleted.
Encryption (encrypt.php):
```
$msg,
'error' => $error
);
// Return a json object to the requesting page
header('Content-type: application/json');
die(json_encode($response_array));
}
// Validation checks
if ($_SERVER['REQUEST_METHOD'] == "POST") {
$continue = true;
// Validation: check if it's an ajax request
if(!isset($_SERVER['HTTP_X_REQUESTED_WITH']) AND strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) != 'xmlhttprequest') {
$continue = false;
response('Hold on there... Submission must be an Ajax POST request.', true);
}
// Validation: check if any of the fields aren't set
if((!isset($_POST['ot_secret']))
|| (!isset($_POST['ot_encrypt_password']))
|| (!isset($_POST['ot_encrypt_password_confirm']))
|| (!isset($_POST['ot_email']))
){
$continue = false;
response('Hold on there... All fields are required.', true);
} else {
$secret = filter_var($_POST['ot_secret'], FILTER_SANITIZE_STRING);
$password = $_POST['ot_encrypt_password'];
$password_confirm = $_POST['ot_encrypt_password_confirm'];
$email = filter_var($_POST['ot_email'], FILTER_SANITIZE_EMAIL);
}
// Validation: check if any of the fields are blank
if((empty($secret)) || (empty($password)) || (empty($password_confirm)) || (empty($email))){
$continue = false;
response('Hold on there... All fields are required.', true);
}
// Validation: check if passwords is long enough
if(strlen($password) Hold
Encryption (encrypt.php):
```
$msg,
'error' => $error
);
// Return a json object to the requesting page
header('Content-type: application/json');
die(json_encode($response_array));
}
// Validation checks
if ($_SERVER['REQUEST_METHOD'] == "POST") {
$continue = true;
// Validation: check if it's an ajax request
if(!isset($_SERVER['HTTP_X_REQUESTED_WITH']) AND strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) != 'xmlhttprequest') {
$continue = false;
response('Hold on there... Submission must be an Ajax POST request.', true);
}
// Validation: check if any of the fields aren't set
if((!isset($_POST['ot_secret']))
|| (!isset($_POST['ot_encrypt_password']))
|| (!isset($_POST['ot_encrypt_password_confirm']))
|| (!isset($_POST['ot_email']))
){
$continue = false;
response('Hold on there... All fields are required.', true);
} else {
$secret = filter_var($_POST['ot_secret'], FILTER_SANITIZE_STRING);
$password = $_POST['ot_encrypt_password'];
$password_confirm = $_POST['ot_encrypt_password_confirm'];
$email = filter_var($_POST['ot_email'], FILTER_SANITIZE_EMAIL);
}
// Validation: check if any of the fields are blank
if((empty($secret)) || (empty($password)) || (empty($password_confirm)) || (empty($email))){
$continue = false;
response('Hold on there... All fields are required.', true);
}
// Validation: check if passwords is long enough
if(strlen($password) Hold
Solution
A few things to suggest:
-
In the following code block, you can return the condition instead of the
if-else
into:
- You should move all your `
returned messages to a config file, rather than keeping them inline.
-
In the following code block, you can return the condition instead of the
if-else
statement:
if(isset($_GET["id"]) && (!empty($_GET["id"]))){
$fromLink = true;
} else {
$fromLink = false;
}
into:
$fromLink = (isset($_GET["id"]) && (!empty($_GET["id"])));
-
I would avoid having the MAGIC NUMBER \$16\$ in the following line, you should move it to a variable and declare it's purpose, just like $iterations:
hash_pbkdf2("sha256", $password, $salt, $iterations, 16);
^
-
In the future, you wouldn't need the === (triple equals) in the following line, you can just use ==, but the whole line is wrong anyway. You test if $continue isset, and then test whether it's true. If it was true, it would be set. Also, comparing a boolean explicitly to ==/=== true` is really bad.if ((isset($continue)) && ($continue === true)) {into:
if ($continue){Code Snippets
if(isset($_GET["id"]) && (!empty($_GET["id"]))){
$fromLink = true;
} else {
$fromLink = false;
}$fromLink = (isset($_GET["id"]) && (!empty($_GET["id"])));hash_pbkdf2("sha256", $password, $salt, $iterations, 16);
^if ((isset($continue)) && ($continue === true)) {if ($continue){Context
StackExchange Code Review Q#96509, answer score: 3
Revisions (0)
No revisions yet.