View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0009274 | GNUnet | other | public | 2024-10-17 13:48 | 2024-10-17 13:50 |
| Reporter | fefe | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | new | Resolution | open | ||
| Summary | 0009274: _make_continuous_arg_copy (in gnunet): integer overflow | ||||
| Description | To be honest, I don't quite understand why you need this function in the first place. You already have a perfectly good copy of argv which you don't need to free at all. Also, the way you allocate the new struct means it is basically read-only as you can't resize any of the strings. If you weren't going to write, why make a copy to begin with? Still, the code also has an integer overflow bug: 1231 for (int i = 0; i < argc; i++) 1232 { 1233 size_t ail = strlen (argv[i]); 1234 1235 GNUNET_assert (SIZE_MAX - 1 - sizeof (char *) > argvsize); 1236 GNUNET_assert (SIZE_MAX - ail > argvsize + 1 + sizeof (char*)); 1237 argvsize += strlen (argv[i]) + 1 + sizeof(char *); 1238 } The assertions prevent integer overflows when counting one pointer and one zero terminated string per element, but you still need to add one char* for the final NULL sentinel: 1239 new_argv = GNUNET_malloc (argvsize + sizeof(char *)); Nobody has checked that this does not overflow. It probably won't, but the same can be said for the previous checks. The kernel has a size limit for argv, I think it's 64k or so. There is one other potential optimization: 1241 for (int i = 0; i < argc; i++) 1242 { 1243 new_argv[i] = p; 1244 strcpy (p, argv[i]); 1245 p += strlen (argv[i]) + 1; 1246 } If you use stpcpy instead of strcpy, you don't have to run strlen again. stpcpy used to be a non-standard extension but has been part of POSIX for a while now. | ||||
| Tags | No tags attached. | ||||
