View Issue Details

IDProjectCategoryView StatusLast Update
0005428GNUnetotherpublic2019-07-24 20:42
ReporterFlorian Dold Assigned Tolurchi  
PrioritynormalSeveritytweakReproducibilityhave not tried
Status closedResolutionfixed 
Product VersionGit master 
Target Version0.11.6Fixed in Version0.11.6 
Summary0005428: some (but not all) uses of strncpy should be removed / replaced by strlcpy
DescriptionWith 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.
TagsNo tags attached.

Activities

nikita

2018-10-21 12:55

developer   ~0013280

Last edited: 2018-10-21 12:57

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

nikita

2018-10-21 18:34

developer   ~0013287

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.

Christian Grothoff

2019-02-12 08:55

manager   ~0013669

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 ;-)).

nikita

2019-02-12 12:02

developer   ~0013679

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.

Christian Grothoff

2019-02-12 14:35

manager   ~0013682

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.

nikita

2019-02-12 14:38

developer   ~0013683

Okay.

lurchi

2019-07-07 12:45

developer   ~0014657

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.

Issue History

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