Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ntlm: avoid malloc(0) for zero length passwords #2054

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -557,7 +557,7 @@ CURLcode Curl_ntlm_core_mk_nt_hash(struct Curl_easy *data,
unsigned char *ntbuffer /* 21 bytes */)
{
size_t len = strlen(password);
unsigned char *pw = malloc(len * 2);
unsigned char *pw = len ? malloc(len * 2) : strdup("");

This comment has been minimized.

Copy link
@jay

jay Nov 4, 2017

Member

I wonder if it would be better practice to just terminate the pw even though technically it doesn't need it

diff --git a/lib/curl_ntlm_core.c b/lib/curl_ntlm_core.c
index 5154949..0a825d8 100644
--- a/lib/curl_ntlm_core.c
+++ b/lib/curl_ntlm_core.c
@@ -557,12 +557,12 @@ CURLcode Curl_ntlm_core_mk_nt_hash(struct Curl_easy *data,
                                    unsigned char *ntbuffer /* 21 bytes */)
 {
   size_t len = strlen(password);
-  unsigned char *pw = malloc(len * 2);
+  unsigned char *pw = malloc((len + 1) * 2);
   CURLcode result;
   if(!pw)
     return CURLE_OUT_OF_MEMORY;
 
-  ascii_to_unicode_le(pw, password, len);
+  ascii_to_unicode_le(pw, password, len + 1);
 
   /*
    * The NT hashed password needs to be created using the password in the

This comment has been minimized.

Copy link
@bagder

bagder Nov 4, 2017

Author Member

Thanks! So you think so? I think that approach invites suspicions that the zero termination is needed there and cast doubts as to why all functions later on are passing in the length separately.

I actually prefer my approach. I tried making the zero-length path avoid the allocation completely but I think that just made the code worse.

This comment has been minimized.

Copy link
@jay

jay Nov 4, 2017

Member

fair enough

CURLcode result;
if(!pw)
return CURLE_OUT_OF_MEMORY;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.