search

smack-team / smack

Smack userspace
GNU Lesser General Public License v2.1
41 stars 33 forks source link

Use strncpy() always copying labels. #42

Closed jarkkojs closed 11 years ago

jarkkojs commented 11 years ago

Defence in depth and a good practice. Places an absolute limit to the length copied.

jarkkojs commented 11 years ago

Last minute fix for the v1.0.2. I think it would be good convention to always stick to strnlen and strncpy even if because of some conditions strcpy might be suitable.

rafal-krypa commented 11 years ago

It's of course your call, your project decision to make. But personally I prefer conscious usage of strcpy() instead of careless strncpy(). The latter can lead to unterminated buffers and truncated input, which isn't any more secure. But using strncpy() as a secondary check surely does no harm, other than additional complexity (having to count the characters once more).

jarkkojs commented 11 years ago

I don't understand your point really to be honest. There's no such thing as sane usage of strcpy() even if you do validation. This is not about adding unnecessary complexity. You are just being ignorant if you consider strcpy() a valid option for anything.

This about minimizing impact of someone taking control of the stack. Relying of having NULL character in stack is much less safer than having explicit limit placed in read-only section.

After these words I can dig one valid point from your opinion. Return value should be checked for strncpy() call and error returned if length has been surpassed. This way we can detect when stack has been modified between label validation and strncpy() call.

jarkkojs commented 11 years ago

I think your point was right that my usage of strncpy() was careless but I think using strcpy() is not the right way to fix it :)