7 ms·
> Safer languages is a good place to start. Could (and should) a safer language pre-empt that you should first check strlen(str) != 0 before doing strcmp? I kn
by georgerobinson 11y ago
> Safer languages is a good place to start.
Could (and should) a safer language pre-empt that you should first check strlen(str) != 0 before doing strcmp? I know of no language which does this.
> Input validation can be made to be machine-verified
Likewise, can you elaborate on this? How does the machine know what type of input this is? If input is tagged by the developer, what's to stop the developer from forgetting to tag the input correctly?
- icebraining 11y agoCould (and should) a safer language pre-empt that you should first check strlen(str) != 0 before doing strcmp? I know of no language which does this. In some languages, you can define a string type that only contains strings of one or more characters. And then you'd need to "convert" a regular string into one of those by using some function that forced you to handle the strlen(str) == 0 case. I believe you can define that in Idris, for example.
- shkkmo 11y agoexcept for strlen(str) != 0 doesn't really do much to fix the issue. At best all it does is mean you have to guess the first character. If you are comparing byte by byte the correct code is: valid = true; for(i = 0; i < strlen(user_token); i++) { if(strlen(token) < i || user_token[i] != token[i]) valid = false; } if(strlen(token) != strlen(user_token)) valid = false; return valid;
- delinka 11y agoLet's make it somewhat more readable: int token_length = strlen(token); int input_length = strlen(input); int match_count = 0; // for constant-time comparison... // Just get this out of the way now; no need to dawdle // (because the length of a token is generally known) if (token_length != input_length) { return false; } // No need to worry about mismatched buffer lengths at this point for(int i=0; i<token_length; i++) { if (token[i] == input[i]) { match_count++; } } // if the same number of characters match, voila! return (match_count == token_length); It takes far too long to understand the intent and verify the correctness of lines like: if(strlen(token) < i || user_token[i] != token[i]) valid = false;
- TheLoneWolfling 11y agoAnd then your compiler will optimize it to this: int token_length = strlen(token); // Just get this out of the way now; no need to dawdle // (because the length of a token is generally known) if (token_length != strlen(input)) { return false; } // No need to worry about mismatched buffer lengths at this point for(int i=0; i<token_length; i++) { if (token[i] != input[i]) { return false; } } return true; And all of a sudden the timing attack is back. You cannot write a constant-time function in portable C / C++, and trying to do so is a fool's game.
- cybojanek 11y agoYes you can - take the hash of the input and token, and compare those. Now you're trying to cause a sha256 collision. Good luck!
- delinka 11y agoYou still have to perform the byte-by-byte comparison and this is where TheLoneWolfling alleges the optimizations kick in and give away secrets. However, from my testing, the variations in timing are unpredictable (I get variations on compare speed on different runs of the same program with the same input) and small and would be swallowed up by variations in network latency.
- rb12345 11y agoYou could probably do that relatively safely with bit operations, but I expect to be proved wrong. Something like this should work: /* Swap char for uint<register_size> for speed */ #define CMP_TYPE char /* Assuming char* sha256(char* input); */ CMP_TYPE* input_hash=(CMP_TYPE*) sha256(input); CMP_TYPE* token_hash=(CMP_TYPE*) sha256(token); /* Precomputed? */ /* max_pos=32 for char on most systems */ int max_pos=256 >> (3 + sizeof(CMP_TYPE)); char cmp=0; for (int i=0;i<max_pos;i++) { cmp = cmp | (token_hash[i] ^ input_hash[i]); } return (cmp != 0); The reason for the CMP_TYPE #define is so that you can optimise the comparison by replacing char with uint32 or uint64.
- Taek 11y agoIn golang, you wouldn't need to write a for loop, you just compare the strings directly. if userToken != token { return false } return true The code is simpler, and the edge case is covered.
- georgerobinson 11y agoWhat about timing attacks?
- Spivak 11y agoWhat do you think this does under the hood? You're basically saying, "why don't you use a correct string comparison function instead of a broken one?" which isn't a particularly valuable insight and has nothing to do with Go. Also, unless you're familiar with how Go implements string comparison you don't know if it's exploitable in other ways like timing attacks. Valve's failure was in testing, not language choice. Also, why not just return userToken == token?
- Taek 11y agoOh right you could just do a 1 liner. Expecting the go builtin functions to be correct is a pretty safe bet. Used by thousands of people and built to stop problems like buffer overflows, vs something you wrote yourself. Using a popular library in a language with unsafe builtin types would achieve a similar effect, as long as the library was code reviewed, etc, but the point is that safer languages don't require you to take that extra step. Use them idiomatically and entire classes of exploits will not be possible through your code. As for timing attacks, if you are making cryptographically sensitive code you really should be using a crypto library and not anything you made yourself.