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

ncat HTTP digest auth does not allow colons #984

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

ncat HTTP digest auth does not allow colons #984

nnposter opened this issue Aug 24, 2017 · 1 comment
Labels

Comments

@nnposter
Copy link

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
Copy link

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
Labels
Projects
None yet
Development

No branches or pull requests

3 participants