strfcpy() ensures it creates a string. But that's not what we want. Here, we just want raw bytes in the output. It's only the input which is a string (password).
strncpy(3) is quite appropriate for this specific use. It's a function that takes a C string as input, and fills a fixed-size buffer with bytes from it, zeroing the unused remainder part of the buffer. Because of this use of strncpy(3), we can remove the memset(3) calls. They're now entirely redundant. (The other branch fills the entire buffer, so it was only meaningful in the branch we're touching.) And because strncpy(3) doesn't need an extra byte for terminating the output (because it doesn't produce a string, unlike strfcpy()), we can remove the +1 in the buffer size calculation. And because we don't use the 'secret' buffer for anything else, we can change its type fro u_char into char, thus avoiding a cast. Now, we can do a raw memcpy(3) of the entire 'secret' buffer into the 'ipad' and 'opad' buffers. Reported-by: Rene Kita <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]> --- Hi, This patch depends on first fixing the bug on the other branch in master. This patch applies on master, not stable. I decided to use strncpy(3) instead of memcpy(3), as it seems more appropriate here. Cheers, Alex imap/auth_cram.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/imap/auth_cram.c b/imap/auth_cram.c index 9844f444..8ea10aad 100644 --- a/imap/auth_cram.c +++ b/imap/auth_cram.c @@ -135,8 +135,8 @@ static void hmac_md5(const char *password, char *challenge, unsigned char *response) { struct md5_ctx ctx; + char secret[MD5_BLOCK_LEN]; unsigned char ipad[MD5_BLOCK_LEN], opad[MD5_BLOCK_LEN]; - unsigned char secret[MD5_BLOCK_LEN+1]; unsigned char hash_passwd[MD5_DIGEST_LEN]; size_t secret_len, chal_len; int i; @@ -153,12 +153,10 @@ static void hmac_md5(const char *password, char *challenge, secret_len = MD5_DIGEST_LEN; } else - strfcpy((char *) secret, password, sizeof(secret)); + strncpy(secret, password, sizeof(secret)); - memset(ipad, 0, sizeof(ipad)); - memset(opad, 0, sizeof(opad)); - memcpy(ipad, secret, secret_len); - memcpy(opad, secret, secret_len); + memcpy(ipad, secret, sizeof(ipad)); + memcpy(opad, secret, sizeof(opad)); for (i = 0; i < MD5_BLOCK_LEN; i++) { base-commit: cc4445225aa6b210a58e2b33e04dde277131f578 prerequisite-patch-id: 22fa16638bc48a96ceca075903856188e13fd49b -- 2.53.0
