Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix errors due to bad strncpy behavour. #1193

Merged
merged 1 commit into from
Dec 14, 2018
Merged

Conversation

tsyesika
Copy link

@tsyesika tsyesika commented Dec 7, 2018

As of GCC 8.1 a compiler error (stringop-truncation) will be raised as strncpy has potentially unsafe behavour, which I think is the case here (not sure if there is a limiting factor on path's length?)

If strncpy's source (path in our case) is longer than the destination the destination will not be null terminated. Which is what sockaddr_un requires of sockaddr_un.sun_path, from the man page unix(7):

pathname: a UNIX domain socket can be bound to a null-terminated filesystem pathname using bind(2).

and

Pathname sockets
When binding a socket to a pathname, a few rules should be observed for maximum portability and ease of coding:

  • The pathname in sun_path should be null-terminated.

By reducing the copy size by one ensures as much as path as can be copied is and pads the rest of destination (the remaining one character) with null bytes, ensuring it's properly null terminated. See man page strcpy(3) for it's behavour:

The strncpy() function is similar, except that at most n bytes of src are copied. Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated.

If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total of n bytes are written.

@tsyesika
Copy link
Author

tsyesika commented Dec 7, 2018

Seems andy beat me to it snabbco#1380

@tsyesika tsyesika closed this Dec 7, 2018
@tsyesika tsyesika reopened this Dec 12, 2018
Copy link
Member

@dpino dpino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

As of GCC 8.1 a compiler error (stringop-truncation) will be raised as
strncpy has potentially unsafe behavour, which is the case here.

If strncpy's source (path in our case) is longer than the destination
the destination will not be null terminated. Which is what sockaddr_un
requires of sockaddr_un.sun_path.

By reducing the copy size by one ensures as much as path as can be
copied is and pads the rest of destination (the remaining one
character) with null bytes, ensuring it's properly null terminated.
@tsyesika tsyesika merged commit 3bba979 into lwaftr Dec 14, 2018
@tsyesika tsyesika deleted the fix-strncpy-errors branch December 19, 2018 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants