View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0005428 | GNUnet | other | public | 2018-08-18 23:22 | 2019-07-24 20:42 |
| Reporter | Florian Dold | Assigned To | lurchi | ||
| Priority | normal | Severity | tweak | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | Git master | ||||
| Target Version | 0.11.6 | Fixed in Version | 0.11.6 | ||
| Summary | 0005428: some (but not all) uses of strncpy should be removed / replaced by strlcpy | ||||
| Description | With strlcpy, we are always guaranteed to have a 0-terminated destination string. This does not hold for strncpy. Some uses of stncpy are legit and necessary (e.g. when copying data to some structs for syscalls), but others might be wrong and dangerous. strlcpy is not portable, so we might need a GNUNET_strlcpy. | ||||
| Tags | No tags attached. | ||||
|
|
strlcpy(3) is in libbsd. We could either bundle this portable strlcpy or have depend on libbsd. Later on we should/must also provide a build-system option to use the system default if it exists (for example on some (all?) of the BSD systems it exists). libbsd can be used as 'libbsd-overlay' by pkg-config, so that bsd/string.h becomes string.h |
|
|
https://salsa.debian.org/dns-team/getdns/blob/debian/master/configure.ac#L1419 might be useful for the general approach. getdns uses strlcpy, uses libbsd if available, and if not uses bundled. I have not read as far to see if they check for general existence of strlcpy on freebsd for example. This just came up while working on libidn+libidn2. |
|
|
I don't think even as an optional dependency we should link against libbsd just for strlcpy. It's not like the performance will be at all relevant for us, so we could just write those 5 lines ourselves and also be more strict: size_t GNUNET_strlcpy(char *dst, const char *src, size_t n) { size_t ret; size_t slen; GNUNET_assert (0 != n); ret = strlen (src); slen = GNUNET_MAX (ret, n - 1); memcpy (dst, src, slen); dst[slen] = '\0'; return ret; } At least that's how I read the man page (modulo the assert on top, that'd be the GNUnet approach: bad errors are punished hard ;-)). |
|
|
Could we arrange it in a way that native strlcpy is used when present? I'm not sure how the different implementations differ at this point, can't be that much. It's in their standard libc's so at least for netbsd this means no breaking changes. |
|
|
I think even on xBSD this would _require_ linking against libbsd (extra dependency at runtime, even if libbsd is always installed, the linker will have extra work), and also the extra checks in my code (n==0) should not be lost. So right now, I don't think that would actually make sense. |
|
|
Okay. |
|
|
All occurrences of strncpy have been replaced by the new GNUNET_strlcpy, except for the following files: exit/gnunet-helper-exit.c dns/gnunet-helper-dns.c transport/gnunet-helper-transport-wlan.c transport/gnunet-helper-transport-bluetooth.c vpn/gnunet-helper-vpn.c Here strncpy is needed because the kernel API or the bluez API require non-null-terminated strings. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-08-18 23:22 | Florian Dold | New Issue | |
| 2018-10-21 12:55 | nikita | Note Added: 0013280 | |
| 2018-10-21 12:57 | nikita | Note Edited: 0013280 | |
| 2018-10-21 18:34 | nikita | Note Added: 0013287 | |
| 2019-02-12 08:55 | Christian Grothoff | Note Added: 0013669 | |
| 2019-02-12 08:56 | Christian Grothoff | Status | new => confirmed |
| 2019-02-12 08:56 | Christian Grothoff | Product Version | => Git master |
| 2019-02-12 08:56 | Christian Grothoff | Target Version | => 0.11.1 |
| 2019-02-12 12:02 | nikita | Note Added: 0013679 | |
| 2019-02-12 14:35 | Christian Grothoff | Note Added: 0013682 | |
| 2019-02-12 14:38 | nikita | Note Added: 0013683 | |
| 2019-02-20 12:28 | Christian Grothoff | Severity | minor => tweak |
| 2019-04-03 12:15 | Christian Grothoff | Target Version | 0.11.1 => |
| 2019-06-05 19:01 | Christian Grothoff | Target Version | => 0.11.6 |
| 2019-06-21 22:50 | Christian Grothoff | Assigned To | => lurchi |
| 2019-06-21 22:50 | Christian Grothoff | Status | confirmed => assigned |
| 2019-07-07 12:45 | lurchi | Note Added: 0014657 | |
| 2019-07-07 12:47 | lurchi | Status | assigned => resolved |
| 2019-07-07 12:47 | lurchi | Resolution | open => fixed |
| 2019-07-07 13:55 | Christian Grothoff | Fixed in Version | => 0.11.6 |
| 2019-07-24 20:42 | Christian Grothoff | Status | resolved => closed |
