[PATCH] Bug 9076 - Whois using a non working host for queries by default
Vito Mulè
mule.vito at gmail.com
Tue Jul 5 08:21:24 UTC 2016
> > + 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/ae2a338a/attachment.html>
More information about the busybox
mailing list