review (XS) for 6765546: Wrong sscanf used to parse CompilerOracle command >= 32 characters could lead to crash

[Vladimir Kozlov]

Tom Rodriguez wrote:
> On Dec 14, 2010, at 4:55 PM, Vladimir Kozlov wrote:
> 
>> It should be in the RANGEBASE definition (not in one sscanf) which is used in other places also:
>>
>> -#define RANGEBASE "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789$_<>" \
>> +#define RANGEBASE "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789$_<>[" \
> 
> RANGEBASE is for class and method names.  The signature line adds to RANGEBASE the characters which are allowed as part of signatures.

You are right. Changes are good.

Thanks,
Vladimir

> 
> tom
> 
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> Sure.  I added that in and tested to make sure array signatures were accepted now.  I've updated the webrev too.  Thanks.
>>> tom
>>> On Dec 14, 2010, at 3:54 PM, Igor Veresov wrote:
>>>> Looks good!
>>>>
>>>> May I also suggest piggybacking that on this change:
>>>>
>>>> @@ -489,7 +489,7 @@ void CompilerOracle::parse_from_line(cha
>>>>    line += bytes_read;
>>>>    // there might be a signature following the method.
>>>>    // signatures always begin with ( so match that by hand
>>>> -    if (1 == sscanf(line, "%*[ \t](%254[);/" RANGEBASE "]%n", sig + 1, &bytes_read)) {
>>>> +    if (1 == sscanf(line, "%*[ \t](%254[[);/" RANGEBASE "]%n", sig + 1, &bytes_read)) {
>>>>
>>>> This basically makes it accept signatures with arrays.
>>>>
>>>> Thanks,
>>>> igor
>>>>
>>>> On 12/14/10 3:30 PM, Tom Rodriguez wrote:
>>>>> http://cr.openjdk.java.net/~never/6765546
>>>>>
>>>>> 6765546: Wrong sscanf used to parse CompilerOracle command>= 32 characters could lead to crash
>>>>> Reviewed-by:
>>>>>
>>>>> The buffer for a sscanf isn't long enough to include the null
>>>>> termination and we're missing a check for unknown commands.  Tested
>>>>> with various command lines.
>