View Issue Details

IDProjectCategoryView StatusLast Update
0005428GNUnetotherpublic2019-04-03 12:15
ReporterFlorian DoldAssigned To 
PrioritynormalSeveritytweakReproducibilityhave not tried
Status confirmedResolutionopen 
Product VersionSVN HEAD 
Target VersionFixed in Version 
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

ng0

2018-10-21 12:55

developer   ~0013280

Last edited: 2018-10-21 12:57

View 2 revisions

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

ng0

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

ng0

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.

ng0

2019-02-12 14:38

developer   ~0013683

Okay.

Issue History

Date Modified Username Field Change
2018-08-18 23:22 Florian Dold New Issue
2018-10-21 12:55 ng0 Note Added: 0013280
2018-10-21 12:57 ng0 Note Edited: 0013280 View Revisions
2018-10-21 18:34 ng0 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 => SVN HEAD
2019-02-12 08:56 Christian Grothoff Target Version => 0.11.1
2019-02-12 12:02 ng0 Note Added: 0013679
2019-02-12 14:35 Christian Grothoff Note Added: 0013682
2019-02-12 14:38 ng0 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 =>