Thanks for the suggestions, cheers.
Signed-off-by: Vito Mule' <[email protected]>
---
networking/whois.c | 56 +++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 48 insertions(+), 8 deletions(-)
diff --git a/networking/whois.c b/networking/whois.c
index bf33033..91663c4 100644
--- a/networking/whois.c
+++ b/networking/whois.c
@@ -28,6 +28,9 @@
#include "libbb.h"
+#define WHOIS_HOST_LEN 128
+#define WHOIS_DOMAIN_LEN 32
+
static void pipe_out(int fd)
{
FILE *fp;
@@ -40,26 +43,63 @@ static void pipe_out(int fd)
*p = '\0';
puts(buf);
}
-
fclose(fp); /* closes fd too */
}
+/* Gets tld from NAME to create whois sever, e.g. tld.whois-servers.net .
*/
+/* Called only from main for each NAME*/
+void whois_host(char* host, char *argv_host, const char *unqualified_host)
+{
+ char domain[WHOIS_DOMAIN_LEN];
+ char* token;
+ char *str_token = strdup(argv_host);
+
+ if (strlen(host) >= 1) {
+ memset(&host[0], 0, WHOIS_HOST_LEN);
+ }
+ token = strtok(str_token, ".");
+ while (token != NULL) {
+ strncpy(domain, token, WHOIS_DOMAIN_LEN);
+ token = strtok(NULL, ".");
+ }
+ strncpy(host, domain, WHOIS_HOST_LEN);
+ strncat(host, unqualified_host, WHOIS_HOST_LEN);
+ 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(WHOIS_HOST_LEN);
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) {
+ dprintf(1, "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:47, walter harms <[email protected]> wrote:
>
>
> Am 05.07.2016 10:38, schrieb Vito Mulè:
> > Now using strndup and removed the sizeof(char) thanks:
> >
> > Signed-off-by: Vito Mule' <[email protected]>
> > ---
> > 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 */
> > }
> >
>
> please add a comment what whois_host() is supposed to do
>
>
> > +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 you are concerned about security ,,
> strlen() will search for \0 so you can safely use strdup()
>
>
> > +
> > + 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));
>
> it will not copy more than strlen(domain) bytes but still
> overwrite a to small destination,
> strncpy(host, domain, host_len);
>
> > + 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);
>
> since c99 we can happily use dprintf().
>
> just my 2 cents,
>
> re,
> wh
>
> > + 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è <[email protected]> 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' <[email protected]>
> >> ---
> >> 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è <[email protected]> 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' <[email protected]>
> >>> ---
> >>> 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 <[email protected]> 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
> >>>>
> >>>
> >>>
> >>
> >
> >
> >
> > _______________________________________________
> > busybox mailing list
> > [email protected]
> > http://lists.busybox.net/mailman/listinfo/busybox
> _______________________________________________
> busybox mailing list
> [email protected]
> http://lists.busybox.net/mailman/listinfo/busybox
>
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
