Fix SHA256 implementation: safer rotate, finalize guard, fread loop, …

hash_file.c

@@ -10,26 +10,31 @@
 
 int main(int argc, char** argv){
     if (argc < 2){
-        printf("Usage: %s [file]\n", argv[0]);
-        return 0;
+        fprintf(stderr, "Usage: %s [file]\n", argv[0]);
+        return 1;
     }
     FILE* file = fopen(argv[1], "rb");
     if (!file){
-        printf("Cannot open file\n");
-        return 0;
+        perror(argv[1]);
+        return 1;
     }
     char buffer[1024];
     size_t size;
     struct sha256_buff buff;
     sha256_init(&buff);
-    while (!feof(file)){
+    while ((size = fread(buffer, 1, sizeof(buffer), file)) > 0){
         /* Hash file by 1kb chunks, instead of loading into RAM at once */
-        size = fread(buffer, 1, 1024, file);
         sha256_update(&buff, buffer, size);
     }
+    if (ferror(file)) {
+        perror("fread");
+        fclose(file);
+        return 1;
+    }
     char hash[65] = {0}; /* hash[64] is null-byte */
     sha256_finalize(&buff);
     sha256_read_hex(&buff, hash);
     printf("%s\n", hash);
+    fclose(file);
     return 0;
 }

sha256.c

@@ -25,6 +25,8 @@
 /* Details of the implementation, etc can be found here: https://en.wikipedia.org/wiki/SHA-2
    See sha256.h for short documentation on library usage */
 
+#include <string.h>
+
 #include "sha256.h"
 
 void sha256_init(struct sha256_buff* buff) {
@@ -51,7 +53,10 @@ static const uint32_t k[64] = {
     0x748f82ee, 0x78a5636f, 0x84c87814, 0x8cc70208, 0x90befffa, 0xa4506ceb, 0xbef9a3f7, 0xc67178f2
 };
 
-#define rotate_r(val, bits) (val >> bits | val << (32 - bits))
+static inline uint32_t rotr(uint32_t x, unsigned n) {
+    n &= 31u;
+    return (x >> n) | (x << (32u - n));
+}
 
 static void sha256_calc_chunk(struct sha256_buff* buff, const uint8_t* chunk) {
     uint32_t w[64];
@@ -64,19 +69,19 @@ static void sha256_calc_chunk(struct sha256_buff* buff, const uint8_t* chunk) {
     }
 
     for (i=16; i<64; ++i){
-        uint32_t s0 = rotate_r(w[i-15], 7) ^ rotate_r(w[i-15], 18) ^ (w[i-15] >> 3);
-        uint32_t s1 = rotate_r(w[i-2], 17) ^ rotate_r(w[i-2], 19) ^ (w[i-2] >> 10);
+        uint32_t s0 = rotr(w[i-15], 7) ^ rotr(w[i-15], 18) ^ (w[i-15] >> 3);
+        uint32_t s1 = rotr(w[i-2], 17) ^ rotr(w[i-2], 19) ^ (w[i-2] >> 10);
         w[i] = w[i-16] + s0 + w[i-7] + s1;
     }
 
     for (i = 0; i < 8; ++i)
         tv[i] = buff->h[i];
 
     for (i=0; i<64; ++i){
-        uint32_t S1 = rotate_r(tv[4], 6) ^ rotate_r(tv[4], 11) ^ rotate_r(tv[4], 25);
+        uint32_t S1 = rotr(tv[4], 6) ^ rotr(tv[4], 11) ^ rotr(tv[4], 25);
         uint32_t ch = (tv[4] & tv[5]) ^ (~tv[4] & tv[6]);
         uint32_t temp1 = tv[7] + S1 + ch + k[i] + w[i];
-        uint32_t S0 = rotate_r(tv[0], 2) ^ rotate_r(tv[0], 13) ^ rotate_r(tv[0], 22);
+        uint32_t S0 = rotr(tv[0], 2) ^ rotr(tv[0], 13) ^ rotr(tv[0], 22);
         uint32_t maj = (tv[0] & tv[1]) ^ (tv[0] & tv[2]) ^ (tv[1] & tv[2]);
         uint32_t temp2 = S0 + maj;
 
@@ -96,7 +101,11 @@ static void sha256_calc_chunk(struct sha256_buff* buff, const uint8_t* chunk) {
 
 void sha256_update(struct sha256_buff* buff, const void* data, size_t size) {
     const uint8_t* ptr = (const uint8_t*)data;
-    buff->data_size += size;
+    if (size > UINT64_MAX - buff->data_size) {
+        buff->data_size = UINT64_MAX;
+    } else {
+        buff->data_size += size;
+    }
     /* If there is data left in buff, concatenate it to process as new chunk */
     if (size + buff->chunk_size >= 64) {
         uint8_t tmp_chunk[64];
@@ -116,38 +125,40 @@ void sha256_update(struct sha256_buff* buff, const void* data, size_t size) {
 
     /* Save remaining data in buff, will be reused on next call or finalize */
     memcpy(buff->last_chunk + buff->chunk_size, ptr, size);
-    buff->chunk_size += size;
+    buff->chunk_size = (uint8_t)(buff->chunk_size + size);
 }
 
 void sha256_finalize(struct sha256_buff* buff) {
-    buff->last_chunk[buff->chunk_size] = 0x80;
-    buff->chunk_size++;
-    memset(buff->last_chunk + buff->chunk_size, 0, 64 - buff->chunk_size);
+    if (buff->chunk_size == sizeof(buff->last_chunk)) {
+        sha256_calc_chunk(buff, buff->last_chunk);
+        buff->chunk_size = 0;
+    }
+
+    buff->last_chunk[buff->chunk_size++] = 0x80;
+    memset(buff->last_chunk + buff->chunk_size, 0, sizeof(buff->last_chunk) - buff->chunk_size);
 
     /* If there isn't enough space to fit int64, pad chunk with zeroes and prepare next chunk */
     if (buff->chunk_size > 56) {
         sha256_calc_chunk(buff, buff->last_chunk);
-        memset(buff->last_chunk, 0, 64);
+        memset(buff->last_chunk, 0, sizeof(buff->last_chunk));
     }
 
     /* Add total size as big-endian int64 x8 */
-    uint64_t size = buff->data_size * 8;
-    int i;
-    for (i = 8; i > 0; --i) {
-        buff->last_chunk[55+i] = size & 255;
-        size >>= 8;
+    uint64_t size_bits = buff->data_size << 3;
+    for (size_t i = 0; i < 8; ++i) {
+        buff->last_chunk[63 - i] = (uint8_t)(size_bits & 255U);
+        size_bits >>= 8;
     }
 
     sha256_calc_chunk(buff, buff->last_chunk);
 }
 
 void sha256_read(const struct sha256_buff* buff, uint8_t* hash) {
-    uint32_t i;
-    for (i = 0; i < 8; i++) {
-        hash[i*4] = (buff->h[i] >> 24) & 255;
-        hash[i*4 + 1] = (buff->h[i] >> 16) & 255;
-        hash[i*4 + 2] = (buff->h[i] >> 8) & 255;
-        hash[i*4 + 3] = buff->h[i] & 255;
+    for (size_t i = 0; i < 8; ++i) {
+        hash[i * 4] = (uint8_t)((buff->h[i] >> 24) & 255U);
+        hash[i * 4 + 1] = (uint8_t)((buff->h[i] >> 16) & 255U);
+        hash[i * 4 + 2] = (uint8_t)((buff->h[i] >> 8) & 255U);
+        hash[i * 4 + 3] = (uint8_t)(buff->h[i] & 255U);
     }
 }