lkml.org 
[lkml]   [2009]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] tracing/filters: use strcmp() instead of strncmp()
Frederic Weisbecker wrote:
> On Mon, Jun 01, 2009 at 01:45:47PM +0800, Li Zefan wrote:
>>>>>> I don't think there's any security issue. It's irrelevant how big the user-input
>>>>>> strings are. The point is those strings are guaranteed to be NULL-terminated.
>>>>>> Am I missing something?
>>>>>>
>>>>>> And I don't think it's necessary to make 2 patches that each patch converts
>>>>>> one strncmp to strcmp. But maybe it's better to improve this changelog?
>>>>> Hmm, you must be right, indeed they seem to be guaranted beeing NULL-terminated
>>>>> strings.
>>>>>
>>>> Sorry, I was wrong. :(
>>>>
>>>> Though the user-input strings are guaranted to be NULL-terminated, strings
>>>> generated by TRACE_EVENT might not.
>>>>
>>>> We define static strings this way:
>>>> TP_struct(
>>>> __array(char, foo, LEN)
>>>> )
>>>> But foo is not necessarily a string, though I doubt someone will use it
>>>> as non-string char array.
>>>
>>> Yeah, but the user defined comparison operand is NULL terminated.
>>> So the strcmp will stop at this boundary.
>>>
>> The user input string is NULL terminated and is limited to MAX_FILTER_STR_VAL,
>> and it's strcmp() not strcpy(), but it's still unsafe. No?
>>
>> cmp = strcmp(addr, pred->str_val);
>>
>> If addr is not NULL-terminated string but char array, and length of
>> str_val > length of addr, then we'll be exceeding the boundary of the
>> array.
>
>
>
> No, once both strings appear to be different, strcmp returns.
> As an example, the generic strcmp in lib/string.c is as follows:
>
> int strcmp(const char *cs, const char *ct)
> {
> signed char __res;
>
> while (1) {
> if ((__res = *cs - *ct++) != 0 || !*cs++)
> break;
> }
> return __res;
> }
>
> Once cs[n] != ct[n], or !cs[n] || !ct[n], strcmp() stops,
> and the x86 implementation does exactly the same.
>
> So I guess it's safe.
>

See this example:

cmp = strcmp(addr, pred->str_val);

length(addr) == 6, strlen(str_val) == 10

123456
addr: abcdef?
^
|
v
str_val: abcdefzzzz\0

or the 2 happen to match even after addr overflowed:

123456
addr: abcdefzzzz?
^
|
v
str_val: abcdefzzzz\0



\
 
 \ /
  Last update: 2009-06-02 02:57    [W:0.216 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site