This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Make strlen range computations more conservative


On 08/02/2018 10:19 PM, Martin Sebor wrote:
> On 08/01/2018 12:55 AM, Bernd Edlinger wrote:
>>> Certainly not every "strlen" has these semantics.  For example,
>>> this open-coded one doesn't:
>>>
>>>    int len = 0;
>>>    for (int i = 0; s.a[i]; ++i)
>>>      ++len;
>>>
>>> It computes 2 (with no warning for the out-of-bounds access).
>>>
>>
>> yes, which is questionable as well, but that happens only
>> if the source code accesses the array via s.a[i]
>> not if it happens to use char *, as this experiment shows:
> 
> Yes, that just happens to be the case with GCC in some
> situations, and not in others.  That's why it shouldn't
> be relied on.
Right.  An access via an ARRAY_REF carries some semantic information
that is not carried by a INDIRECT_REF.   However, we may at times lower
an ARRAY_REF to an INDIRECT_REF -- and I believe we've had code in the
front-ends to go the other way.

> 
>> The point I make is that it is impossible to know where the function
>> is inlined, and if the original code can be broken in surprising ways.
>> And most importantly strlen is often used in security relevant ways.
> 
> Code that's concerned with security or safety (which should
> be all of it) needs to follow the basic rules of the language.
> Calling strlen() on a char[4] argument expecting it to return
> a value larger than 3 as an indication that the array isn't
> nul-terminated is not a secure coding practice -- it's a plain
> old bug.  Don't take my word for it -- read any of the secure
> coding standards: CEERT STR32-C. Do not pass a non-null-terminated
> character sequence to a library function that expects a string,
> CWE-170: Improper Null Termination, OWASP String Termination
> Error.  This is elementary material that shouldn't need
> explaining.
I couldn't agree more with this.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]