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
