3 ms·

s/int/size_t/ and check for overflow.

by js2 7y ago

GuB-42 7y ago
Totally right about size_t, my bad, hopefully, the compiler will raise a warning. As for integer overflow, I don't actually know how to handle it properly. In normal conditions, it is unlikely to be a problem. If the two strings can fit in memory, the sum of their size should fit in a size_t, but I agree that making such assumptions can be a bad idea. Maybe the best way is to limit the size of the input strings to a reasonable value. That would prevent many out of memory situations too and potential DoS too.
- js2 7y ago
  > As for integer overflow, I don't actually know how to handle it properly. Something like: if (str1 && str2) { size_t len1 = strlen(str1); size_t len2 = strlen(str2); size_t buf_len = len1 + len2 + 1; if (len1 < buf_len && len2 < buf_len) { char *buf = malloc(buf_len); if (buf) { memcpy(buf, str1, len1); memcpy(buf + len1, str2, buf_len - len1 - 1); buf[buf_len - 1] = '\0'; } } } (I probably made a mistake above.) As you suggest, you'll probably run out of memory before you'll overflow, so in reality, you want to check len1 and len2 are some sane value, but of course, library functions don't usually have that luxury. Take a hint from git: #define unsigned_add_overflows(a, b) \ ((b) > maximum_unsigned_value_of_type(a) - (a)) if (unsigned_add_overflows(extra, 1) || unsigned_add_overflows(sb->len, extra + 1)) die("you want to use way too much memory"); https://github.com/git/git/blob/6d5b26420848ec3bc7eae46a7ffa54f20276249d/git-compat-util.h https://github.com/git/git/blob/6d5b26420848ec3bc7eae46a7ffa... https://github.com/git/git/blob/9d418600f4d10dcbbfb0b5fdbc71d509e03ba719/strbuf.c#L90 https://github.com/git/git/blob/9d418600f4d10dcbbfb0b5fdbc71... > making such assumptions can be a bad idea It's always a bad idea, especially in an unsafe language. Never trust user input.

s/int/size_t/ and check for overflow.

by js2 7y ago

s/int/size_t/ and check for overflow.

Totally right about size_t, my bad, hopefully, the compiler will raise a warning. As for integer overflow, I don't actually know how to handle it properly. In normal conditions, it is unlikely to be a problem. If the two strings can fit in memory, the sum of their size should fit in a size_t, but I agree that making such assumptions can be a bad idea. Maybe the best way is to limit the size of the input strings to a reasonable value. That would prevent many out of memory situations too and potential DoS too.

js2 7y ago
> As for integer overflow, I don't actually know how to handle it properly. Something like: if (str1 && str2) { size_t len1 = strlen(str1); size_t len2 = strlen(str2); size_t buf_len = len1 + len2 + 1; if (len1 < buf_len && len2 < buf_len) { char *buf = malloc(buf_len); if (buf) { memcpy(buf, str1, len1); memcpy(buf + len1, str2, buf_len - len1 - 1); buf[buf_len - 1] = '\0'; } } } (I probably made a mistake above.) As you suggest, you'll probably run out of memory before you'll overflow, so in reality, you want to check len1 and len2 are some sane value, but of course, library functions don't usually have that luxury. Take a hint from git: #define unsigned_add_overflows(a, b) \ ((b) > maximum_unsigned_value_of_type(a) - (a)) if (unsigned_add_overflows(extra, 1) || unsigned_add_overflows(sb->len, extra + 1)) die("you want to use way too much memory"); https://github.com/git/git/blob/6d5b26420848ec3bc7eae46a7ffa54f20276249d/git-compat-util.h https://github.com/git/git/blob/6d5b26420848ec3bc7eae46a7ffa... https://github.com/git/git/blob/9d418600f4d10dcbbfb0b5fdbc71d509e03ba719/strbuf.c#L90 https://github.com/git/git/blob/9d418600f4d10dcbbfb0b5fdbc71... > making such assumptions can be a bad idea It's always a bad idea, especially in an unsafe language. Never trust user input.