fit remote code execution in history and template imports functionality#1625
fit remote code execution in history and template imports functionality#1625Ngai-E wants to merge 4 commits into
Conversation
| if (!isset($_POST['token'])) { | ||
| error_log('WARNING: A POST request detected with no csrf token found'); | ||
| die('Authentication failed.'); | ||
| } else if (!hash_equals(hash_hmac('sha256', '/letter.php.theform', $_SESSION['token']), $_POST['token'])) { |
There was a problem hiding this comment.
Why is verifyCsrfTokenAndCompareHash not used here rather?
| if (!isset($_POST['token'])) { | ||
| error_log('WARNING: A POST request detected with no csrf token found'); | ||
| die('Authentication failed.'); | ||
| } else if (!(CsrfToken::verifyCsrfToken($_POST['token'])) { |
There was a problem hiding this comment.
You're missing a closing parenthesis here too
| // Function to verify a csrf token using with second token | ||
| function verifyCsrfTokenAndCompareHash($token, $secondToken) | ||
| { | ||
| if (hash_equals(hash_hmac('sha256', $secondToken, $_SESSION['token']), $token) { |
There was a problem hiding this comment.
You're missing a closing parenthesis here too
There was a problem hiding this comment.
Its necessary to type cast the inputs before supplying to hash_hmac, interesting things might occur if that is not done ( type juggling vulnerability ), if $secondToken is a user controlled input, if an array is passed, hash_hmac will return null.
For more details:
https://www.youtube.com/watch?v=MpeaSNERwQA
There was a problem hiding this comment.
thanks @naveen17797 I will add that too.
| if (!isset($_POST['token'])) { | ||
| error_log('WARNING: A POST request detected with no csrf token found'); | ||
| die('Authentication failed.'); | ||
| } else if (!(CsrfToken::verifyCsrfToken($_POST['token'])) { |
There was a problem hiding this comment.
You're missing a closing parenthesis here
There was a problem hiding this comment.
thanks @maggienegm. I am guessing you fixed in here PR #1634?? have not checked yet. If you did then I can just close this.
There was a problem hiding this comment.
Yes @Ngai-E , I fixed the closing parenthesis issues there
|
@Ngai-E can you rebase thanks |
4 participants
No description provided.