Skip to content

Commit c475e5b

Browse files
lhchavezethomson
authored andcommitted
Make NTLMClient Memory and UndefinedBehavior Sanitizer-clean
This change makes the code pass the libgit2 tests cleanly when MSan/UBSan are enabled. Notably: * Changes malloc/memset combos into calloc for easier auditing. * Makes `write_buf` return early if the buffer length is empty to avoid arithmetic with NULL pointers (which UBSan does not like). * Initializes a few arrays that were sometimes being read before being written to.
1 parent 4ea5beb commit c475e5b

File tree

1 file changed

+8
-11
lines changed

1 file changed

+8
-11
lines changed

src/ntlm.c

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,9 @@ ntlm_client *ntlm_client_init(ntlm_client_flags flags)
4747
{
4848
ntlm_client *ntlm = NULL;
4949

50-
if ((ntlm = malloc(sizeof(ntlm_client))) == NULL)
50+
if ((ntlm = calloc(1, sizeof(ntlm_client))) == NULL)
5151
return NULL;
5252

53-
memset(ntlm, 0, sizeof(ntlm_client));
54-
5553
ntlm->flags = flags;
5654

5755
if ((ntlm->hmac_ctx = ntlm_hmac_ctx_init()) == NULL ||
@@ -260,6 +258,9 @@ static inline bool write_buf(
260258
const unsigned char *buf,
261259
size_t len)
262260
{
261+
if (!len)
262+
return true;
263+
263264
if (out->len - out->pos < len) {
264265
ntlm_client_set_errmsg(ntlm, "out of buffer space");
265266
return false;
@@ -648,13 +649,11 @@ int ntlm_client_negotiate(
648649
return -1;
649650
}
650651

651-
if ((ntlm->negotiate.buf = malloc(ntlm->negotiate.len)) == NULL) {
652+
if ((ntlm->negotiate.buf = calloc(1, ntlm->negotiate.len)) == NULL) {
652653
ntlm_client_set_errmsg(ntlm, "out of memory");
653654
return -1;
654655
}
655656

656-
memset(ntlm->negotiate.buf, 0, ntlm->negotiate.len);
657-
658657
if (!write_buf(ntlm, &ntlm->negotiate,
659658
ntlm_client_signature, sizeof(ntlm_client_signature)) ||
660659
!write_int32(ntlm, &ntlm->negotiate, 1) ||
@@ -1122,7 +1121,7 @@ static bool generate_ntlm2_challengehash(
11221121
static bool generate_lm2_response(ntlm_client *ntlm,
11231122
unsigned char ntlm2_hash[NTLM_NTLM2_HASH_LEN])
11241123
{
1125-
unsigned char lm2_challengehash[16];
1124+
unsigned char lm2_challengehash[16] = {0};
11261125
size_t lm2_len = 16;
11271126
uint64_t local_nonce;
11281127

@@ -1177,7 +1176,7 @@ static bool generate_ntlm2_response(ntlm_client *ntlm)
11771176
uint32_t signature;
11781177
uint64_t timestamp, nonce;
11791178
unsigned char ntlm2_hash[NTLM_NTLM2_HASH_LEN];
1180-
unsigned char challengehash[16];
1179+
unsigned char challengehash[16] = {0};
11811180
unsigned char *blob;
11821181

11831182
if (!generate_timestamp(ntlm) ||
@@ -1334,13 +1333,11 @@ int ntlm_client_response(
13341333
return -1;
13351334
}
13361335

1337-
if ((ntlm->response.buf = malloc(ntlm->response.len)) == NULL) {
1336+
if ((ntlm->response.buf = calloc(1, ntlm->response.len)) == NULL) {
13381337
ntlm_client_set_errmsg(ntlm, "out of memory");
13391338
return -1;
13401339
}
13411340

1342-
memset(ntlm->response.buf, 0, ntlm->response.len);
1343-
13441341
if (!write_buf(ntlm, &ntlm->response,
13451342
ntlm_client_signature, sizeof(ntlm_client_signature)) ||
13461343
!write_int32(ntlm, &ntlm->response, 3) ||

0 commit comments

Comments
 (0)