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

Vito Mulè mule.vito at gmail.com
Tue Jul 5 08:38:19 UTC 2016


Now using strndup and removed the sizeof(char) thanks:

Signed-off-by: Vito Mule' <mulevito at gmail.com>
---
 networking/whois.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 43 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;
+       char *str_token = strndup(argv_host, strlen(argv_host));
+
+       if (strlen(host) >= 1) {
+               memset(&host[0], 0, strlen(host));
+       }
+       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);
        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 09:21, Vito Mulè <mule.vito at gmail.com> wrote:

> > > +       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 actually I misread this, I'll add some parenthesis
> Thanks
>
>
>
> 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);
>
> On 5 July 2016 at 09:15, Vito Mulè <mule.vito at gmail.com> wrote:
>
>> 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/0a885d16/attachment-0001.html>


More information about the busybox mailing list