C String strncmp difficulty

[Disch]

There might be some communication issues here.

The point I think vlad is trying to make is that using strlen() on the value you pass to strncmp is likely a bug because it does not do what you expect.

strncmp( "foo", "foobar", strlen("foo") );  // <- probably wrong 

This is bad because strlen gives you the length of "foo", which is 3. Therefore strncmp will only compare 3 characters. Since the first 3 characters of each string are identical... strncmp will return 0 indicating they match, even though they clearly do not.

The 3rd parameter should not be the length of the string, but should be the size of the buffer. The whole point of the param is to prevent buffer overflow... so giving it a strlen value completely defeats the point.


EDIT:

@ciphermagi
isValid = strncmp(str1, str2, MAX(strlen(str1), strlen(str2));

This is bad code. The whole point of strncmp is to be "safer" by stopping from going out of bounds of a given buffer.

In that regard strlen() is "unsafe" because it iterates over the entire string without regard to where the buffer actually ends.

So in effect... doing strlen on BOTH strings is even less safe and slower than just doing a regular, 'deprecated' strcmp.

Putting the calls in a MAX macro is even worse still because it will ensure strlen is called multiple times on the same string, resulting in the string being unnecessarily parsed twice.


Do yourself a favor and just use strcmp.

[ciphermagi]

Yes, but he's stuck on the idea that HAVE to use the smallest input, while I'm trying to show him that it needs to use the largest input.

And for some reason he's saying that strlen can ONLY take the shortest value.

Take a look at the bottom of the last page, if you want to see what I would actually use to compare two dynamic strings which could possibly be of any size.

[vlad from moscow]

ciphermagi (793)

Look again at my post. There should have been 9 steps, because the string passed to srtlen was "exit toll", not "exit". That's not even complete code, because I would actually define it more like this:

#define MAX(x, y) ((x) >= (y) ? (x) : (y))
..
..
..
isValid = strncmp(str1, str2, MAX(strlen(str1), strlen(str2));

Bravo!:) Instead of using simply strcmp you will at least three times call strlen and one more strncmp.

I am sorry but this code simply is awful.:)

[Disch]

My edit got ninja'd. Please see my edit above.

[ciphermagi]

omg you just don't quit. 1 + 1 = 2

That's not the point, Disch. With this code you have a buffer defined. You don't buffer overrun because the program actually handles the whole thing. Since your strings can't exceed the buffer, no, the trick doesn't work, because you can't cause undefined behavior in the program.

[Disch]

That's not the point, Disch.

You said not to use strcmp because it doesn't do buffer checking.... and then you give an example using strncmp .... but you use strlen which also doesn't do buffer checking... effectively destroying any safety you gain from using strncmp.

So yes... I must be missing the point. Can you clarify it for me?

From all I can see... the code you are using is no safer than strcmp... but is 4x as slow.

[vlad from moscow]

@ciphermagi

It seems that you do not understand what I am saying but all what you are trying to do now is to simulate function strcmp instead of simply using it.

[ciphermagi]

Disch, part of the point I was making is that since you force a buffer definition, even though it's an awkward one, you're preventing the buffer overrun attacks. I was trying to use a simple example, but for some reason vlad seems to think that it's impossible for the function to work, so I guess the examples got out of hand. strlen isn't subject to those attacks because of two factors, and only the fact that these are both present make it immune to the same attack: One is that there is only one string involved, and the other is that the function terminates at '\0', which is the end of the buffer.

Mostly in the last two pages I was trying to get past vlad's idea that no matter what you put in as your buffer length, the strncmp function would always fail to perform an actual comparison.

[cire]

That's not the point, Disch. With this code you have a buffer defined. You don't buffer overrun because the program actually handles the whole thing. Since your strings can't exceed the buffer, no, the trick doesn't work, because you can't cause undefined behavior in the program.

I think you're missing the point. A buffer overrun wasn't possible in the original code using strcmp. So, using strncmp to avoid the buffer overflow that was already impossible wasn't necessary (and as we can see from your machinations, more error prone.)

Anyway, it seems to me the way to stop a user from exploiting a buffer overrun is in the input extraction and validation. Not in a general purpose comparison function.

[edit: And, I think this thread may serve as a nice example for preferring std::string in C++!]

[cire]

strlen isn't subject to those attacks because of two factors, and only the fact that these are both present make it immune to the same attack: One is that there is only one string involved, and the other is that the function terminates at '\0', which is the end of the buffer.

The only reason to usestrncmp over strcmp is if you are unsure whether those nul characters are in place or you wish to compare only a portion of the strings. If the nul character is out of place, you're equally screwed with strlen.

[Disch]

Late reply

Disch, part of the point I was making is that since you force a buffer definition, even though it's an awkward one, you're preventing the buffer overrun attacks.

What do you mean by "buffer definition"? I think that's where I'm misunderstanding you.

Anyway... my point is that using strlen in conjuction with strncmp does absolutely nothing to prevent buffer overrun (because strlen is unaware of the buffer size). In fact it is functionally identical to a normal strcmp. The only difference is strlen+strncmp is likely to be slower, because the string has to be iterated over multiple times.

So really if you're going to give strncmp a value you calculated with strlen... it completely defeats the point of using strncmp in the first place, and you're better off just using strcmp.