>On Mon, Oct 10, 2011 at 8:44 PM, Zdenek Styblik
><zdenek.styb...@gmail.com> wrote:
>[...]
>> It is good you're using strtol() function rather than atoi(), because
>> it does (kind of) validation for you.
>> However I think you should check whether the last processed character
>> was '\0' and whether there
>> was not overflow.
>>
>>>> + retry = (uint8_t)strtol(optarg, NULL, 0);
>>
>> ~~~
>> #include <errno.h>
>> [...]
>> char *end_ptr = 0;
>> long retry_long = 0;
>> errno = 0;
>> retry_long = strtol(optarg, &end_ptr, 0);
>> if (errno != 0) { /* overflow occured */ }
>> if (*end_ptr != '\0' || retry < 0) { /* something like 123abc was
>> supplied rather than int */ }
>> /* all good */
>> retry = (uint8_t)retry_long;
>> ~~~
>>
>> Checking `retry < 0' might be pointless if retry will be of type of
>> unsigned long.
>>
>> Anyway, my point is:
>> * as of now, it is possible to input number greater than unsigned long
>> and there is no check
>> whether user did or did not. Entering such input value will cause
>> overflow and value of retry_long
>> is going to be "random"(honestly, I don't know whether values after
>> overflow can be predicted
>> or not. But that's beyond the point here)
>[...]
>
>Hello Harshad,
>
>one thing just pop-ed up in my head about this thing. And sadly it led
>to another.
>First of all, I did little speech about overflow yet there is no check
>whether long fits
>into unsigned char.
>
>So I've decided to rewrite your patch only to find out
>'lib/ipmi_main.c' is full of
>these. I'll be working on solution, because it seems the best way to
>fix this is to
>write a function.
>Of course if somebody is willing to fix this, be my guest.
>I will apply your patch and continue from there, or shouldn't I?
>
>Best regards,
>Z.
>
>>> -----Original Message-----
>>> From: Zdenek Styblik [mailto:zdenek.styb...@gmail.com]
>>> Sent: Monday, October 10, 2011 10:27 PM
>>> To: Harshad Prabhu (hprabhu)
>>> Cc: ipmitool-devel@lists.sourceforge.net
>>> Subject: Re: [Ipmitool-devel] Regarding Retries/Timeout for
>LAN/LANPLUS interface
>>>
>>> On Mon, Oct 10, 2011 at 5:32 PM, Harshad Prabhu (hprabhu)
>>> <hpra...@cisco.com> wrote:
>>> [...]
>>>> + /* Retry and Timeout */
>>>> + case 'R':
>>>> + retry = (uint8_t)strtol(optarg, NULL, 0);
>>>> + break;
>>>> + case 'N':
>>>> + timeout = (uint8_t)strtol(optarg, NULL, 0);
>>>> + break;
>>>
>>> Hello,
>>>
>>> it seems you're not worried it is possible to overflow `retry' and
>>> `timeout' which will lead to unexpected behavior.
>>>
>>> Regards,
>>> Zdenek
>>>
>>
Hi Zdenek,
Sorry could not reply to your previous mail.
Yes when I added the patch, had used the same way as it was already there in
the file.
Please add the patch contents and make the changes for complete file.
Thanks once again.
Best Regards,
Harshad
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2d-oct
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
