Dear OpenBSD developers,

I am writing to briefly inform you of a small bug that we have
encountered in your implementation of SipHash as we utilized your code
in our nameserver Knot DNS (thank you for that!).

Specifically the issue is that the line src/lib/libc/hash/siphash.c:107
reads

memcpy(&ctx->buf[used], ptr, len);

while it should read

memcpy(&ctx->buf, ptr, len);

The reasoning behind this change in the function SipHash_Update is as
follows: the data passed to the SipHash_Update function in ptr are
partitioned to chunks of size sizeof(ctx->buf) and processed. If there
are less than sizeof(ctx->buf) bytes left at the end, this remainder is
just copied to the buffer and not processed, which is what the mentioned
memcpy does. However, after processing all the previous blocks, the
buffer is empty and so the write should start the beginning of ctx->buf.
Instead the write starts at ctx->buf + used, where used is the number of
leftover bytes from the previous update, which were however already
processed on lines 83-95 and after that the content of the variable used
is no longer relevant and should not be used.

If only a single call to SipHash_Update is performed or if the size of
processed data is a multiple of sizeof(ctx->buf), this bug does nothing.
However when we performed several updates of various lengths, the data
written to ctx->buf were too long and the call to memcpy overwrote other
data, which led to various unexpected behavior.

Thank you for reading this report, hopefully you will find it useful.


Best regards,

Mark Karpilovskij, developer at CZ.NIC labs

https://www.knot-dns.cz/



Reply via email to