BLAKE3-team / BLAKE3

the official Rust and C implementations of the BLAKE3 cryptographic hash function
Apache License 2.0
4.71k stars 315 forks source link

C function "chunk_state_update" modifies function parameters #360

Closed gruenich closed 4 months ago

gruenich commented 8 months ago

Something is fishy with the C function chunk_state_update in blake3.c. The function parameters input and input_len are modified. The first one might be a missing reference, but the second one?

Cppcheck warns about this issue:

/home/gruenich/BLAKE3/c/blake3.c:138:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg]
  input += take;
  ^
/home/gruenich/BLAKE3/c/blake3.c:139:3: style: Assignment of function parameter has no effect outside the function. [uselessAssignmentArg]
  input_len -= take;
  ^
/home/gruenich/BLAKE3/c/blake3_impl.h:173:60: style: Parameter 'cv_words' can be declared as const array [constParameter]
INLINE void store_cv_words(uint8_t bytes_out[32], uint32_t cv_words[8]) {
                                                           ^
/home/gruenich/BLAKE3/c/blake3.c:447:57: style: Parameter 'new_cv' can be declared as const array [constParameter]
INLINE void hasher_push_cv(blake3_hasher *self, uint8_t new_cv[BLAKE3_OUT_LEN],
                                                        ^
/home/gruenich/BLAKE3/c/blake3.c:138:9: style: Variable 'input' is assigned a value that is never used. [unreadVariable]
  input += take;
        ^
/home/gruenich/BLAKE3/c/blake3.c:139:13: style: Variable 'input_len' is assigned a value that is never used. [unreadVariable]
  input_len -= take;

I don't understand what the function is actually supposed to do, so I did not work on a patch.

sneves commented 8 months ago

blake3.c is more or less a line-by-line rewrite of the Rust code, as far as I understand. The code you mention corresponds to this line in the original.

In the original there's a later assertion that checks whether input is empty, so that operation is not entirely useless. In the C version there is not, so those lines serve no purpose.