Messages in this thread | | | Date | Mon, 01 Jun 2009 13:45:47 +0800 | From | Li Zefan <> | Subject | Re: [PATCH 2/2] tracing/filters: use strcmp() instead of strncmp() |
| |
>>>> 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.
> > >> Dynamic string is fine, because assign_str() makes it NULL-terminated. >> >> So we can use strcmp() for dynamic strings, but we'd better use strncmp() for >> static string. >> >> > > >
| |