View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0009274 | GNUnet | other | public | 2024-10-17 13:48 | 2024-10-23 21:45 |
Reporter | fefe | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | new | Resolution | open | ||
Target Version | 0.22.2 | ||||
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. | ||||
|
My guess from the API documentation is that when we supported Windows natively, this command converted the non-UTF-8 arguments to UTF-8 arguments in the newly allocated buffer. Since we dropped W32 support some time ago, this function should probably be a NOP. |
Date Modified | Username | Field | Change |
---|---|---|---|
2024-10-17 13:48 | fefe | New Issue | |
2024-10-17 13:49 | fefe | Summary | _make_continuous_arg_copy: integer overflow => _make_continuous_arg_copy (in gnunet): integer overflow |
2024-10-17 13:50 | fefe | Project | Taler => GNUnet |
2024-10-23 13:20 | schanzen | Target Version | git (master) => 0.22.2 |
2024-10-23 21:45 | schanzen | Note Added: 0023581 |