Improved security of OptInHandler.

mailgun · Jan 24, 2014 · 717cf5f · 717cf5f
1 parent 6bfebad
commit 717cf5f
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 11 deletions.
diff --git a/src/Mailgun/Lists/OptInHandler.php b/src/Mailgun/Lists/OptInHandler.php
@@ -18,18 +18,28 @@ function __construct(){
 	}
 
 	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;
 	}
 }
diff --git a/tests/Mailgun/Tests/Lists/OptInHandlerTest.php b/tests/Mailgun/Tests/Lists/OptInHandlerTest.php
@@ -16,17 +16,17 @@ public function setUp(){
 
 	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);
 	}
 }