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

ncat HTTP digest auth does not allow colons #984

Closed
nnposter opened this Issue Aug 24, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@nnposter

nnposter commented Aug 24, 2017

The following patch allows ncat to properly process HTTP digest passwords that are either empty or contain colons.

--- a/ncat/ncat_connect.c
+++ b/ncat/ncat_connect.c
@@ -399,12 +399,13 @@
 
         /* Split up the proxy auth argument. */
         proxy_auth = Strdup(o.proxy_auth);
-        username = strtok(proxy_auth, ":");
-        password = strtok(NULL, ":");
+        username = proxy_auth;
+        password = strchr(proxy_auth, ':');
         if (password == NULL) {
             free(proxy_auth);
             return NULL;
         }
+        *password++ = '\0';
         response_hdr = http_digest_proxy_authorization(challenge,
             username, password, "CONNECT", sock_to_url(o.target,o.portno));
         if (response_hdr == NULL) {
--- a/ncat/ncat_proxy.c
+++ b/ncat/ncat_proxy.c
@@ -888,12 +888,13 @@
 
         /* Split up the proxy auth argument. */
         proxy_auth = Strdup(o.proxy_auth);
-        username = strtok(proxy_auth, ":");
-        password = strtok(NULL, ":");
+        username = proxy_auth;
+        password = strchr(proxy_auth, ':');
         if (password == NULL) {
             free(proxy_auth);
             return 0;
         }
+        *password++ = '\0';
         ret = http_digest_check_credentials(username, "Ncat", password,
             request->method, credentials);
         free(proxy_auth);

Please let me know if you have any questions or concerns. Otherwise I will commit the patch in a few weeks.

@cldrn cldrn added the Ncat label Aug 26, 2017

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Sep 11, 2017

I am nearly always in favor of a patch that removes an invocation of strtok. I do think that *password++ = '\0'; is slightly confusing, since it involves dereferencing and pointer arithmetic in an lvalue. For clarity, I would prefer:

*password = '\0';
password++;

Though since this of course means the same thing, you may commit whichever you like.

dmiller-nmap commented Sep 11, 2017

I am nearly always in favor of a patch that removes an invocation of strtok. I do think that *password++ = '\0'; is slightly confusing, since it involves dereferencing and pointer arithmetic in an lvalue. For clarity, I would prefer:

*password = '\0';
password++;

Though since this of course means the same thing, you may commit whichever you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment