Encryption-decryption method

Question

My encryption algorithm

I'm using this algorithm in order to encrypt notes users save on my site:

  function CasualPassword($lenght=527){
    $available_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890"; 
    $password = ""; 
    for($i = 0; $i<$lenght; $i++){
        $password .= substr($available_chars,rand(0,strlen($available_chars)-1),1);
    } 
    return $password; 
  }
$key = CasualPassword();
$string = nl2br($_POST['nota']);
for ($i = 0; $i < strlen($string); $i++ ) {
   $temp = $string[$i] ^ $key[$i % strlen($key)];
   $crypt .= str_pad( dechex( ord( $temp ) ), 2, 0, STR_PAD_LEFT);
}
$encryptednote = $key.'__'.$crypt;

So if I encrypted "Code review is awesome" I'd get something like this:

CB9A25T2g2kgSmeUAtFu6h2vOeVPMLOONMOFGh8nYhJrKoCKXH6TpwF4QaJcmYrzk66rd3U3ZekjJmuhuq1Zc6TtWdL923ybytIRldKB9ulHWWoGCfgfa1OxTavKTklHUuZ7Oj8MTjTq69x9dUd5KsrdIdnbDPiY09UOhRa22uixtVvMqPH18zTGS7hWMGuLEEZmW1eNkCBTYwgECo6AdOGvLoIXu4jz90cUz7ia0juYWRdZ6YZYmgKyKTM6MvvKuNfkKDAKnuiABBZ8ZzVCVg2fzDbBzMMjarKkvKuZ6SZ8Uz5uoQwAwofmr8ngJ1GjhQKTb6sWOIUi9PSmfFmStI5bFi8PYEz7V0Qw59JIHdrFsAsOFUhzdbQj0OyYSnVnnlZikjrrCYxvYfAe1hTT0j39xrXFUst9UJg5sNmvmZgA5hevFyEEX8DLoaR0JA8dJeX2r0mTZJUTqzFvnJ1BH4MwTdvcet7nNTe6THOsRjC8ZHtDRkEZdRLwv7PPpL44fYyzfDiwnkuCseF__002d5d24124731440e571c473a1e45343611351a5b0d

Or this:

6sa9JEQHpMmvwapMLUPlVKbVSSXgbbPWJpVSulZZyCKkc1xoKCPfMr1fAOBbpQyz7JHqvhGjjQKCH9qRB6xOnotn3vEiC3X9CfpxCvXmChpJfVbhROv53eivPmTRfrQhM6UPrs3uQi4w4VTLjEGcioNyRhGxPWUbSpr0XktoInaC0tOixUBW0OKRwQEBnZd6EL6cvpG5Tg7SQK2Df3afHlw4216fPYbUjWxVC4PVaMorXfv3XLXewKYoBTJQrllR8YnAS4WtGAadgLW4XuYJ5wy6G9MxjyFhMthfm4Os5pMa2iUOSsYMfNIlLUZvIfSVO112uFkovwEmUZ2nHPznd9OoSoZbJsxYjoQTt19fnCH9b9ljObMrZbWsFwKPf8oEl5XVvV1YY9X180aLam41EPjKCtahrElT99puUgIsegjbf0n7mgwrwV2yFRVwW7GV6vFPRO9wuiNPh1LKX92JUs8ZASMWPSrKExaWBYIv7VBeMNojLfsfOp6O9HlNaTXeGx2IM04JV6nhISH__751c055c6a37343e19281a561e12502c3b3023033b2e

My decryption algorithm

$separate = explode("__",$encryptednote);
$key =$separate[0];
$crypt = $separate[1];
$cnt = 0;
for ($i = 0; $i < strlen( $crypt ); $i+=2){
    $temp = chr( hexdec( substr( $crypt, $i, 2) ) );
    $string .= $temp ^ $key[$cnt % strlen($key)];
    $cnt++;
}

Complete code

<?php
include("../co.php");
include("config.php");
if($_GET['del']!="" and is_numeric($_GET['del'])){
  $user = addslashes($_SESSION['user']);
  $drop = "DELETE FROM notes WHERE id='".$_GET['del']."' and username='".$user."'";
mysqli_query($connect,$drop);
$_SESSION['download']="0";
$_SESSION['downloaded']="";
echo '<script>
$("#princ").html("Note deleted.");
 $("#princ").load("../includes/note.php", function (responseText, textStatus, req) {
   if (textStatus == "error") {
      $("#princ").html("An error occurred");
   }
});
  setTimeout(function() {
$("#note").load("../includes/note.php?load=true", function (responseText, textStatus, req) {
   if (textStatus == "error") {
      $("#princ").html("An error occurred");
   }
});
}, 10000);
 </script>';
die();
}
if($_POST['nota']!=""){//critta e inserisci nel db
  function CasualPassword($lenght=527){
    $available_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890"; 
    $password = ""; 
    for($i = 0; $i<$lenght; $i++){
        $password .= substr($available_chars,rand(0,strlen($available_chars)-1),1);
    } 
    return $password; 
  }
$key = CasualPassword();
$string = nl2br($_POST['nota']);
for ($i = 0; $i < strlen($string); $i++ ) {
   $temp = $string[$i] ^ $key[$i % strlen($key)];
   $crypt .= str_pad( dechex( ord( $temp ) ), 2, 0, STR_PAD_LEFT);
}
$encryptednote = $key.'__'.$crypt;
$query = "INSERT INTO  `notes` (
`username` ,
`nota`
)
VALUES (
'$user',  '$encryptednote'
);";
mysqli_query($connect,$query);
$_SESSION['download']="0";
$_SESSION['downloaded']="";
echo '<script>
 $("#note").html("Loading...");
 $("#note").load("../includes/note.php?load=true", function (responseText, textStatus, req) {
   if (textStatus == "error") {
      $("#princ").html("An error occurred.");
   }
});
 function update() {
  $.get("../includes/note.php?load=true", function(data) {
    $("#note").html(data);
  });
}
window.setTimeout(update, 3000);
 </script>';
}
else if($_POST['nota']=="" and $_GET['load']==""){echo '<script>
$(document).ready(function() {
  $("#go").click(function(){
  $("#note").html("Please wait...");
$.ajax({
url:"../includes/note.php",
type: "POST",
data: $("#notes").serialize(),
   success: function(msg)
      {
        $("#note").html(msg);
      },
      error: function()
      {
        alert("Error!");
      }
    });
  });
});
 $("#note").html("Loading...");
 $("#note").load("../includes/note.php?load=true", function (responseText, textStatus, req) {
   if (textStatus == "error") {
      $("#princ").html("An error occurred.");
   }
});
function update() {
  $.get("../includes/note.php?load=true", function(data) {
    $("#note").html(data);
  });
}
</script>
<form method="post" action="../includes/note.php" id="notes">
<textarea name="nota" id="nota" style="width:100%; height:20%;"></textarea>
<input type="button" id="go" value="Save">
</form>';}
if($_GET['load']=="true"){echo '<h3>Your notes</h3>';
//seleziona tutte le note
  $user = addslashes($_SESSION['user']);
  $download = addslashes($_SESSION['download']);
$query = mysqli_query($connect,"SELECT * FROM notes WHERE username='".$user."' AND id>'".$download."' ORDER BY id DESC LIMIT 0,50");
while($note = mysqli_fetch_assoc($query)){
$separate = explode("__",$encryptednote);
$key =$separate[0];
$crypt = $separate[1];
$cnt = 0;
for ($i = 0; $i < strlen( $crypt ); $i+=2){
    $temp = chr( hexdec( substr( $crypt, $i, 2) ) );
    $string .= $temp ^ $key[$cnt % strlen($key)];
    $cnt++;
}
echo $string.'<br><button class="opzione" id="elimina'.$note['id'].'" onclick="$(\'#elimina'.$note['id'].'\').hide(); $(\'#confermaz'.$note['id'].'\').show();">Delete</button><button class="opzione" id="confermaz'.$note['id'].'" style="display:none;"><a href="javascript:apriLink(\'../includes/note.php?del='.$note['id'].'\')">Confirm</a></button><hr>';
if($note['id']>$_SESSION['download'] or $_SESSION['download']==""){$_SESSION['download']=$note['id'];}
$_SESSION['downloaded'] .= $stringa.'<br><button class="opzione" id="elimina'.$note['id'].'" onclick="$(\'#elimina'.$note['id'].'\').hide(); $(\'#confermaz'.$note['id'].'\').show();">Delete</button><button class="opzione" id="confermaz'.$note['id'].'" style="display:none;"><a href="javascript:apriLink(\'../includes/note.php?del='.$note['id'].'\')">Confirm</a></button><hr>';
$stringa="";
}
echo $_SESSION['downloaded'];
echo '<script>
window.setTimeout(update, 3000); </script>';
die();
}
?>
<div id="note"></div>

My question

Is this method to encrypt/decrypt a string safe?

Could you please include the complete code? As your code is in italian, it is hard to read without function declarations and an example on how those functions are called. And to answer your question: most likely it is not safe. It's always good to program things yourself to learn, but for a live site, I would suggest existing solutions. — tim, Sep 3 '14 at 16:03
Thanks for your comment :) I translated variable names into English, anyway I'm posting the complete code (just the time to translate it too). Could you give me some examples of encryption algorithms that I could use? — user46302, Sep 3 '14 at 16:06
Wow, looking at your revised question after posting my answer has made my brain wobble! SQL INJECTION ALERT! NOT SAFE CODE! never trust user inputted variables see Tim's post for more info. — CodeX, Sep 3 '14 at 16:33
The problem here is that this sort of code is what you usually get back from google when searching for MySQLi code / wrappers. I started off coding using similar practices, very unsafe — CodeX, Sep 3 '14 at 16:36
@CodeX Yeah, I didn't know that addslashes() wasn't enough sure and that XSS attacks could affect other users than the one who saved the note (which should be private, so I thought nobody could be attacked by running a code of an other user); the deletion query was secured with is_numeric, and the user session could only be alphanumeric. To be honest (really), this code is one of my first that I've done all by myself some time ago, but I had never thought (but looks like I should have) of improving it until now. Thanks again for your answer :) — user46302, Sep 3 '14 at 18:25

tim · Accepted Answer · 2014-09-04 09:58:06Z

Security

Encryption

This is not secure.

You are sending out the key with the encrypted message, so anyone getting their hands on your code (us right now, for example), can easily decode any message. Security through obscurity is not real security.

And as I mentioned in the comments, coding for experience is good, but don't code your own hashing or encryption mechanisms.

Better Encryption

Since you asked for existing alternatives: use mcrypts mcrypt_encrypt and mcrypt_decrypt.

I think that the example from the PHP website linked above isn't all that bad. Here is another example for how to use (somewhat insecure) encryption in PHP. It clearly separates between encrption and decryption (which the PHP website example does not), but it uses CFB instead of CBC (which is fine for that example, but for your case CBC would be better).

As mentioned here in the comments as well as on the PHP website, both examples are not secure. They do not check the integrity or authenticity, and they are not protected against padding oracle attacks.

To prevent this, use Encrypt-then-MAC. Here you can find a description of the necessary steps for secure encryption, and here is an encryption implementation that looks ok.

SQL injection

Please read up on SQL injection and how to prevent them (use prepared statements). Your code is (or might be) vulnerable right now. Here:

$drop = "DELETE FROM notes WHERE id='".$_GET['del']."' and username='".$user."'";

And also here:

$query = "INSERT INTO  `notes` (
[...]

XSS

You are also open to XSS attacks here:

echo $string

See here how to prevent XSS.

Other

use camelCase for function and variable names (so CasualPassword becomes casualPassword.
consistency: either always use $i++ in loops or $i+=2.
consistency: put spaces before and after equals (key =$separate[0];).

@Angelo sure :) Please also see my update, your encryption is not the only security issue (and not even the worst. You should really fix the sql injection and xss issues as soon as possible). — tim, Sep 3 '14 at 16:31
In relation to your last edit: the first query is not run if ID is not numeric and $user is escaped with addslashes(); $utente represent a SESSION which is set by the server and I thought it wasn't necessary to escape it because usernames can be only alphanumeric; and didn't think that encrypted strings with my algorithm could contain "'". Am I wrong? Thanks for pointing out the possibility of XSS attacks, I'm fixing it right now :) — user46302, Sep 3 '14 at 16:32
You might have a point with SESSION, I don't know how that value is set. To be save I would still use prepared statements (it's good practice to do so even if no injection is possible right now, who knows what other values might be added in the future). — tim, Sep 3 '14 at 16:42

CodeX · Answer 2 · 2014-09-03 21:21:06Z

First off it looks like your trying to encrypt / decrypt a password with that code..

http://php.net/manual/en/function.password-hash.php

There are great functions available for encryption / decryption HERE

Anyway, this is how i do it:

"# === WARNING ===

# Resulting cipher text has no integrity or authenticity added
# and is not protected against padding oracle attacks."

    private function setupCipher(){
        $iv_size = mcrypt_get_iv_size(MCRYPT_RIJNDAEL_128, MCRYPT_MODE_CBC);

        return array(
            pack('H*', "bcb04b7e103a0cd8b54763051cef08bc55abe029fdebae5e1d417e2ffb2a00a3"),
            $iv_size, 
            mcrypt_create_iv($iv_size, MCRYPT_RAND)
        );
    } 

    public function decrypt($string){
        list($key, $iv_size) = $this->setupCipher();

        $string = base64_decode($string);
        $iv_dec = substr($string, 0, $iv_size);
        $ciphertext_dec = substr($string, $iv_size);
        return trim(mcrypt_decrypt(MCRYPT_RIJNDAEL_128, $key, $ciphertext_dec, MCRYPT_MODE_CBC, $iv_dec), "\0");
    }

    public function encrypt($string) {
        list($key, $iv_size, $iv) = $this->setupCipher();

        $string = $iv . mcrypt_encrypt(MCRYPT_RIJNDAEL_128, $key, $string, MCRYPT_MODE_CBC, $iv);
        return trim(base64_encode($string), "\0");
    }

Thanks for your answer, I'll take a look to mcrypt and use your code :) — user46302, Sep 3 '14 at 16:23
1) Lack MAC enables padding oracles and similar attacks 2) Corrupts messages that end with a \0 since you're using a lossy padding. Consider PKCS#7 padding. 3) Prefer MCRYPT_DEV_URANDOM over MCRYPT_RAND. — CodesInChaos, Sep 3 '14 at 21:09
Very good point I'll update the answer to reflect what you said there, I had to add \0 as I was getting unusual characters added to my string. — CodeX, Sep 3 '14 at 21:20

asked	1 year ago
viewed	2793 times
active	1 year ago

current community

your communities

more stack exchange communities

Encryption-decryption method

My encryption algorithm

My decryption algorithm

Complete code

My question

2 Answers 2

Security

Other

Your Answer

Linked

Hot Network Questions

current community

your communities

more stack exchange communities

Encryption-decryption method

My encryption algorithm

My decryption algorithm

Complete code

My question

2 Answers 2

Security

Other

Your Answer

Sign up or log in

Post as a guest

Linked

Related

Hot Network Questions