From c2056c6cbf93bb545d99d408aeae7d787822a035 Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Thu, 10 Oct 2019 08:10:57 +0100
Subject: [PATCH] SHA: Fix some implementation-defined behaviour (#343)

* SHA: Fix negative base shift UB

* SHA: Avoid signed integer overflow

We do cast from uint32_t to int32_t but that should be OK everywhere.

* SHA: Always use portable arithmetic right shift
---
 Source/sha.cpp | 35 ++++++++++++++++++++++++++++++++---
 Source/sha.h   |  5 -----
 structs.h      |  4 ++--
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/Source/sha.cpp b/Source/sha.cpp
index bf0d62da6..6cb9bacf4 100644
--- a/Source/sha.cpp
+++ b/Source/sha.cpp
@@ -1,7 +1,36 @@
 #include "diablo.h"
 
+#include <cstdint>
+
 DEVILUTION_BEGIN_NAMESPACE
 
+// Diablo's "SHA1" is different from actual SHA1 in that it uses arithmetic
+// right shifts (sign bit extension).
+//
+// Note that:
+// 1. Tere is a C++20 proposal to make right shift always defined
+//    as an arithmetic shift with sign extension:
+//    http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0907r1.html
+// 2. Modern C++ compilers (GCC 8+, Clang 7+) compile the portable code
+//    to the same assembly as the implementation defined `value >> bits`:
+//    https://stackoverflow.com/a/53766752
+
+namespace {
+
+std::uint32_t asr(std::int32_t value, std::int32_t amount)
+{
+	return value < 0 ? ~(~value >> amount) : value >> amount;
+}
+
+/*
+ * Diablo-"SHA1" circular left shift.
+ */
+std::uint32_t SHA1CircularShift(std::uint32_t bits, std::uint32_t word) {
+	return (word << bits) | asr(word, 32 - bits);
+}
+
+} // namespace
+
 SHA1Context sgSHA1[3];
 
 void SHA1Clear()
@@ -50,9 +79,9 @@ void SHA1Input(SHA1Context *context, const char *message_array, int len)
 
 void SHA1ProcessMessageBlock(SHA1Context *context)
 {
-	int i, temp;
-	int W[80];
-	int A, B, C, D, E;
+	DWORD i, temp;
+	DWORD W[80];
+	DWORD A, B, C, D, E;
 
 	DWORD *buf = (DWORD *)context->buffer;
 	for (i = 0; i < 16; i++)
diff --git a/Source/sha.h b/Source/sha.h
index e4902e1e2..e97f540dd 100644
--- a/Source/sha.h
+++ b/Source/sha.h
@@ -2,11 +2,6 @@
 #ifndef __SHA_H__
 #define __SHA_H__
 
-/*
- *  Define the SHA1 circular left shift macro
- */
-#define SHA1CircularShift(bits, word) \
-	(((word) << (bits)) | ((word) >> (32 - (bits))))
 #define SHA1HashSize 20
 
 //sha
diff --git a/structs.h b/structs.h
index fbc36fd5a..68a3d230b 100644
--- a/structs.h
+++ b/structs.h
@@ -1433,8 +1433,8 @@ typedef struct PATHNODE {
 //////////////////////////////////////////////////
 
 typedef struct SHA1Context {
-	int state[5];
-	int count[2];
+	DWORD state[5];
+	DWORD count[2];
 	char buffer[64];
 } SHA1Context;