On: What I learned from my first C coding challenge

[raverbashing]

Very interesting. The author at first seemed not that familiar with C, but in the end he got a good result

Depends also if there's pre validation (even with C you're never sure, so for example, reading that he used strcpy makes my head go "security hole", but this is a proof-of-concept so it's ok)

The best thing to keep in mind is that "There is no such thing as the fastest code"

Edit: strcmp to strcpy (or any str C function)

[tptacek → raverbashing]

Reading strcmp() makes you think "security hole"? Why?

[Someone → tptacek]

It does not apply here, but strcmp is, due to the early bailout that any sane implementation uses, suspectible to timing attacks, as in http://www.cs.rice.edu/~scrosby/slides/SCISS-latency.1.ppt.

[tptacek → Someone]

A timing attack against URL query strings as processed by a normalizing cache proxy?

[masterzora → tptacek]

Quoth the parent: "It does not apply here"

[tptacek → masterzora]

Sure. The flaw here isn't strcmp. In fact, most crypto compares don't use strcmp, even in naive code; an HMAC-SHA1 MAC, for instance, is an array of 8-bit bytes, not the hex string that programs encode them into for human consumption. "memcmp" is the normal culprit.

Timing attacks aren't a flaw in memcmp or strcmp. Touching every byte of a string is stupid behavior in the overwhelming majority of cases.

[Xuzz → tptacek]

Unless, of course, you're Nintendo: http://wiibrew.org/wiki/Signing_bug

[tptacek → Xuzz]

Good find. Again, notice how the problem here isn't strcmp or it's timing behavior; it's mistakenly using ASCIIZ strings to hold ciphertext.

[raverbashing → tptacek]

Actually I meant strcpy, but the point stands

Point your string function to a non-null terminated byte buffer for example. At the very least you can crash the app

What's recommended (one of the recommendations) is to use the 'n' functions like strncmp that takes a maximum size.

[tptacek → raverbashing]

I'm sorry, I should have been more direct. No, I meant to say. Using "strcmp" in a typical C program is not a security flaw. It was clear to me you were thinking of "strcat" or "strcpy".

Using strncmp in this situation makes very little sense and is probably more dangerous. The lengths given to strncmp() are inevitably going to be derived from something else that requires a NUL terminator. Meanwhile, strncmp() leaves you open to logic flaws where you compare too few bytes.

[jparishy → tptacek]

It doesn't take the length/amount of characters to compare as an argument relying on the null terminator, meaning it's susceptible to a buffer overflow attack.

Moral of the story, if you're going to use C strings, use the strn* variants.

[tptacek → jparishy]

No, strcmp is not susceptible to buffer overflow attacks.

[jparishy → tptacek]

https://buildsecurityin.us-cert.gov/bsi-rules/home/g1/847-BS...

If passed an unterminated string, the function will fail at least. How much you could exploit from that, I guess I exaggerated.

[tptacek → jparishy]

That's not a buffer overflow.

This whole subthread of picking on the guy's implementation because of "strcmp" is pretty silly. There are times where strcpy() is safe to use, but most of the time it's a red flag. There are conceivably times when strcmp() is unsafe to use, but to a professional reviewer, it is very rarely a red flag.

I should have just come right out and said that, rather than begging for the rationale for picking on strcmp().

[adsr → jparishy]

I prefer strcpy to strncpy, and make sure that the string will fit before the call. I use strncpy if I want to copy n characters from s2 to s1, strncpy can give a false sense of security imo since it may not add a terminating zero, for example using strncpy with strlen. The BSD strlcpy does always null terminate the string, but it's non standard.

[varrunr → tptacek]

strcmp() should be the least of security concerns. The input buffer in test.c is directly passed to the url_sort() function => Buffer overflow attacks.