From 717cf5faed07234443b1802ba81fa2580ef090dd Mon Sep 17 00:00:00 2001 From: Travis Swientek Date: Fri, 24 Jan 2014 12:32:51 -0800 Subject: [PATCH] Improved security of OptInHandler. --- src/Mailgun/Lists/OptInHandler.php | 26 +++++++++++++------ .../Mailgun/Tests/Lists/OptInHandlerTest.php | 6 ++--- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/Mailgun/Lists/OptInHandler.php b/src/Mailgun/Lists/OptInHandler.php index fc538a1..ba59d3c 100644 --- a/src/Mailgun/Lists/OptInHandler.php +++ b/src/Mailgun/Lists/OptInHandler.php @@ -18,18 +18,28 @@ class OptInHandler{ } public function generateHash($mailingList, $secretAppId, $recipientAddress){ - $concatStrings = $secretAppId . "" . $recipientAddress; - return urlencode(base64_encode(json_encode(array('s' => hash('md5', $concatStrings), 'l' => $mailingList, 'r' => $recipientAddress)))); + $innerPayload = array('r' => $recipientAddress, 'l' => $mailingList); + $encodedInnerPayload = base64_encode(json_encode($innerPayload)); + + $innerHash = hash_hmac("sha1", $encodedInnerPayload, $secretAppId); + $outerPayload = array('h' => $innerHash, 'p' => $encodedInnerPayload); + + return urlencode(base64_encode(json_encode($outerPayload))); } public function validateHash($secretAppId, $uniqueHash){ - $urlParameters = json_decode(base64_decode(urldecode($uniqueHash))); - $concatStrings = $secretAppId . "" . $urlParameters->r; - - if($urlParameters->s == hash('md5', $concatStrings)){ - $returnArray = array('recipientAddress' => $urlParameters->r, 'mailingList' => $urlParameters->l); - return $returnArray; + $decodedOuterPayload = json_decode(base64_decode(urldecode($uniqueHash)), true); + + $decodedHash = $decodedOuterPayload['h']; + $innerPayload = $decodedOuterPayload['p']; + + $decodedInnerPayload = json_decode(base64_decode($innerPayload), true); + $computedInnerHash = hash_hmac("sha1", $innerPayload, $secretAppId); + + if($computedInnerHash == $decodedHash){ + return array('recipientAddress' => $decodedInnerPayload['r'], 'mailingList' => $decodedInnerPayload['l']); } + return false; } } \ No newline at end of file diff --git a/tests/Mailgun/Tests/Lists/OptInHandlerTest.php b/tests/Mailgun/Tests/Lists/OptInHandlerTest.php index 46bb24f..735d1b5 100644 --- a/tests/Mailgun/Tests/Lists/OptInHandlerTest.php +++ b/tests/Mailgun/Tests/Lists/OptInHandlerTest.php @@ -16,17 +16,17 @@ class OptInHandler extends \Mailgun\Tests\MailgunTestCase{ public function testReturnOfGenerateHash(){ $generatedHash = $this->optInHandler->generateHash('mytestlist@example.com', 'mysupersecretappid', 'testrecipient@example.com'); - $knownHash = "eyJzIjoiOGM2NmVmYzYwNzhmNGVkYjFkZGJiY2RhM2M2MmMzMTQiLCJsIjoibXl0ZXN0bGlzdEBleGFtcGxlLmNvbSIsInIiOiJ0ZXN0cmVjaXBpZW50QGV4YW1wbGUuY29tIn0%3D"; + $knownHash = "eyJoIjoiMTllODc2YWNkMWRmNzk4NTc0ZTU0YzhjMzIzOTNiYTNjNzdhNGMxOCIsInAiOiJleUp5SWpvaWRHVnpkSEpsWTJsd2FXVnVkRUJsZUdGdGNHeGxMbU52YlNJc0ltd2lPaUp0ZVhSbGMzUnNhWE4wUUdWNFlXMXdiR1V1WTI5dEluMD0ifQ%3D%3D"; $this->assertEquals($generatedHash, $knownHash); } public function testGoodHash(){ - $validation = $this->optInHandler->validateHash('mysupersecretappid', 'eyJzIjoiOGM2NmVmYzYwNzhmNGVkYjFkZGJiY2RhM2M2MmMzMTQiLCJsIjoibXl0ZXN0bGlzdEBleGFtcGxlLmNvbSIsInIiOiJ0ZXN0cmVjaXBpZW50QGV4YW1wbGUuY29tIn0%3D'); + $validation = $this->optInHandler->validateHash('mysupersecretappid', 'eyJoIjoiMTllODc2YWNkMWRmNzk4NTc0ZTU0YzhjMzIzOTNiYTNjNzdhNGMxOCIsInAiOiJleUp5SWpvaWRHVnpkSEpsWTJsd2FXVnVkRUJsZUdGdGNHeGxMbU52YlNJc0ltd2lPaUp0ZVhSbGMzUnNhWE4wUUdWNFlXMXdiR1V1WTI5dEluMD0ifQ%3D%3D'); $this->assertArrayHasKey('recipientAddress', $validation); $this->assertArrayHasKey('mailingList', $validation); } public function testBadHash(){ - $validation = $this->optInHandler->validateHash('mybadsecretappid', 'eyJzIjoiOGM2NmVmYzYwNzhmNGVkYjFkZGJiY2RhM2M2MmMzMTQiLCJsIjoibXl0ZXN0bGlzdEBleGFtcGxlLmNvbSIsInIiOiJ0ZXN0cmVjaXBpZW50QGV4YW1wbGUuY29tIn0%3D'); + $validation = $this->optInHandler->validateHash('mybadsecretappid', 'eyJoIjoiMTllODc2YWNkMWRmNzk4NTc0ZTU0YzhjMzIzOTNiYTNjNzdhNGMxOCIsInAiOiJleUp5SWpvaWRHVnpkSEpsWTJsd2FXVnVkRUJsZUdGdGNHeGxMbU52YlNJc0ltd2lPaUp0ZVhSbGMzUnNhWE4wUUdWNFlXMXdiR1V1WTI5dEluMD0ifQ%3D%3D'); $this->assertFalse($validation); } }