[#6518] CodeQL alert SM02200: Weak hmacs in microsoft/microsoft/botbu…

…ilder-dotnet/botbuilder-dotnet (#6535) * Replace HMAC for SHA2 in FacebookClientWrapper * Update Facebook Functional Tests
microsoft · Oct 27, 2022 · 91e2905 · 91e2905
1 parent d8a7684
commit 91e2905
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 10 deletions.
diff --git a/FunctionalTests/Microsoft.Bot.Builder.FunctionalTests/FacebookChatTests.cs b/FunctionalTests/Microsoft.Bot.Builder.FunctionalTests/FacebookChatTests.cs
@@ -91,11 +91,10 @@ private string CreateHubSignature(string bodyMessage)
         {
             var hashResult = string.Empty;
 
-            using (var hmac = new System.Security.Cryptography.HMACSHA1(Encoding.UTF8.GetBytes(_appSecret)))
+            using (var hmac = new System.Security.Cryptography.HMACSHA256(Encoding.UTF8.GetBytes(_appSecret)))
             {
-                hmac.Initialize();
                 var hashArray = hmac.ComputeHash(Encoding.UTF8.GetBytes(bodyMessage));
-                var hash = $"SHA1={BitConverter.ToString(hashArray).Replace("-", string.Empty)}";
+                var hash = BitConverter.ToString(hashArray).Replace("-", string.Empty);
 
                 hashResult = hash;
             }

diff --git a/libraries/Adapters/Microsoft.Bot.Builder.Adapters.Facebook/FacebookClientWrapper.cs b/libraries/Adapters/Microsoft.Bot.Builder.Adapters.Facebook/FacebookClientWrapper.cs
@@ -115,16 +115,12 @@ public virtual bool VerifySignature(HttpRequest request, string payload)
 
             var expected = request.Headers["x-hub-signature"].ToString().ToUpperInvariant();
 
-#pragma warning disable CA5350 // Facebook uses SHA1 as cryptographic algorithm.
-            using (var hmac = new HMACSHA1(Encoding.UTF8.GetBytes(_options.FacebookAppSecret)))
+            using (var hmac = new HMACSHA256(Encoding.UTF8.GetBytes(_options.FacebookAppSecret)))
             {
-                hmac.Initialize();
                 var hashArray = hmac.ComputeHash(Encoding.UTF8.GetBytes(payload));
-                var hash = $"SHA1={BitConverter.ToString(hashArray).Replace("-", string.Empty)}";
-
+                var hash = BitConverter.ToString(hashArray).Replace("-", string.Empty);
                 return expected == hash;
             }
-#pragma warning restore CA5350 // Facebook uses SHA1 as cryptographic algorithm.
         }
 
         /// <summary>

diff --git a/tests/Adapters/Microsoft.Bot.Builder.Adapters.Facebook.Tests/FacebookWrapperTests.cs b/tests/Adapters/Microsoft.Bot.Builder.Adapters.Facebook.Tests/FacebookWrapperTests.cs
@@ -41,7 +41,7 @@ public void VerifySignatureShouldThrowErrorWithNullRequest()
         [Fact]
         public void VerifySignatureShouldReturnTrueWithValidRequestHash()
         {
-            const string requestHash = "SHA1=70C0E1B415F16D986EB839144FC85A941A5899C7";
+            const string requestHash = "13870D954C7CB3A6725C7C8DC58260E6EEE77D538DAFEA1A3703DCC2AE21E97F";
             var facebookWrapper = new FacebookClientWrapper(_testOptions);
             var request = new Mock<HttpRequest>();
             var stringifyBody = File.ReadAllText(Directory.GetCurrentDirectory() + @"/Files/RequestResponse.json");