[PATCH] Bug 9076 - Whois using a non working host for queries by default

[Vito Mulè]

Hello,
I forgot to send the last patch in, that is attached on the bug. I could
use strdup if you think it's better.

>> stopped using strcpy,
>>Why?

Buffer overflow?

If the destination string of a strcpy() is not large enough, then anything
might happen. Overflowing fixed-length string buffers is a favorite cracker
technique for taking complete control of the machine. Any time a program
reads or copies data into a buffer, the program first needs to check that
there's enough space. This may be unnecessary if you can show that overflow
is impossible, but be careful: programs can get changed over time, in ways
that may make the impossible possible.

> > +       strncpy(str_token, argv_host, query_len+1);
>
> Ignoring the way you've taken 3 lines to say strdup() (and your
> sizeof(char) is only applying to the 1, not query_len+1, but it's ok
> because sizeof(char) is a constant 1 on every system linux has ever
> supported)...

Yeah true, but it helps readability and it's also a good habit to have when
you are using malloc with other types, at least for me.



whois_fix_default_server_V4

Final Patch:
Dealing with names that are too long, respecting the length used by
original whois, to avoid segfaults.

Cheers



Signed-off-by: Vito Mule' <mulevito at gmail.com>
---
 networking/whois.c | 52
+++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/networking/whois.c b/networking/whois.c
index bf33033..b23a40f 100644
--- a/networking/whois.c
+++ b/networking/whois.c
@@ -44,22 +44,60 @@ static void pipe_out(int fd)
        fclose(fp); /* closes fd too */
 }

+void whois_host(char* host, char *argv_host, const char *unqualified_host)
+{
+       char domain[69];
+       char* token;
+       size_t query_len = strlen(argv_host);
+       char *str_token = malloc(query_len+1 * sizeof(char));
+
+       if (strlen(host) >= 1) {
+               memset(&host[0], 0, strlen(host));
+       }
+       strncpy(str_token, argv_host, query_len);
+       token = strtok(str_token, ".");
+       while (token != NULL) {
+               strncpy(domain, token, strlen(token)+1);
+               token = strtok(NULL, ".");
+       }
+       strncpy(host, domain, strlen(domain));
+       strncat(host, unqualified_host, strlen(unqualified_host));
+        free(str_token);
+}
+
 int whois_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int whois_main(int argc UNUSED_PARAM, char **argv)
 {
+
+       char *host = malloc(128 * sizeof(char));
        int port = 43;
-       const char *host = "whois-servers.net";
+       const char *unqualified_host = ".whois-servers.net";

        opt_complementary = "-1:p+";
        getopt32(argv, "h:p:", &host, &port);
-
        argv += optind;
-       do {
-               int fd = create_and_connect_stream_or_die(host, port);
-               fdprintf(fd, "%s\r\n", *argv);
-               pipe_out(fd);
+
+       if (strlen(host) < 1) {
+               do {
+                       if (strlen(*argv) >= 67) {
+                               fprintf(stdout, "Invalid request: %s\n",
*argv);
+                               return EXIT_FAILURE;
+                       }
+                       whois_host(host, *argv, unqualified_host);
+                       int fd = create_and_connect_stream_or_die(host,
port);
+                       fdprintf(fd, "%s\r\n", *argv);
+                       pipe_out(fd);
+               }
+               while (*++argv);
+               free(host);
+       } else {
+               do {
+                       int fd = create_and_connect_stream_or_die(host,
port);
+                       fdprintf(fd, "%s\r\n", *argv);
+                       pipe_out(fd);
+               }
+               while (*++argv);
        }
-       while (*++argv);

        return EXIT_SUCCESS;
 }



On 5 July 2016 at 03:36, Rob Landley <rob at landley.net> wrote:

> On 07/04/2016 02:47 PM, Vito Mulè wrote:
> > stopped using strcpy,
>
> Why?
>
> > +    size_t query_len = strlen(argv_host);
> > +       char *str_token = malloc(query_len+1 * sizeof(char));
>
> > +       strncpy(str_token, argv_host, query_len+1);
>
> Ignoring the way you've taken 3 lines to say strdup() (and your
> sizeof(char) is only applying to the 1, not query_len+1, but it's ok
> because sizeof(char) is a constant 1 on every system linux has ever
> supported)...
>
> My question is that you're effectively doing:
>
>   strncpy(str_token, argv_host, strlen(argv_host)+1);
>
> So how is this better than strcpy()?
>
> > +       strncpy(host, domain, strlen(domain));
>
> Here you're literally doing that. Why?
>
> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20160705/941d30a0/attachment-0001.html>