From 91e290512384f01b67f652eff0dc13f9cbc6c645 Mon Sep 17 00:00:00 2001 From: Cecilia Avila <44245136+ceciliaavila@users.noreply.github.com> Date: Thu, 27 Oct 2022 15:31:33 -0300 Subject: [PATCH] [#6518] CodeQL alert SM02200: Weak hmacs in microsoft/microsoft/botbuilder-dotnet/botbuilder-dotnet (#6535) * Replace HMAC for SHA2 in FacebookClientWrapper * Update Facebook Functional Tests --- .../FacebookChatTests.cs | 5 ++--- .../FacebookClientWrapper.cs | 8 ++------ .../FacebookWrapperTests.cs | 2 +- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/FunctionalTests/Microsoft.Bot.Builder.FunctionalTests/FacebookChatTests.cs b/FunctionalTests/Microsoft.Bot.Builder.FunctionalTests/FacebookChatTests.cs index f44069ea38..4ea153e370 100644 --- 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 index 01b148543f..e66df63a14 100644 --- 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. } /// diff --git a/tests/Adapters/Microsoft.Bot.Builder.Adapters.Facebook.Tests/FacebookWrapperTests.cs b/tests/Adapters/Microsoft.Bot.Builder.Adapters.Facebook.Tests/FacebookWrapperTests.cs index d4a3557d2c..b7ea05909c 100644 --- 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(); var stringifyBody = File.ReadAllText(Directory.GetCurrentDirectory() + @"/Files/RequestResponse.json");