[vlc-devel] [PATCH] control: rc: avoid stringop-truncation warning

[Alexandre Janniaux]

To add more precision, the warning says that the use of strncpy could
be dangerous as the size specified is the same as the buffer size,
because it could lead to non-null terminated string if given a string
of size this buffer size + null character or more.

It didn't raise issue because the null character is enforced, and
copying one byte less change nothing to what is really copied to the
final string, as the null character will be truncated from the copied
string if needed.

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Jan 20, 2020 at 02:51:03PM +0100, Alexandre Janniaux wrote:
> Hi,
>
> There should be no functional differences as the null character is
> forced in the end of the path. Copying one meaningful character less
> would happend as soon as the path is already too long, and otherwise
> only the strncpy null character is expected to be truncated, but is
> ensured by the code following.
>
> The documentation state that sun_path should contain a null-terminated
> pathname so it doesn't look wrong to enforce the last \0 byte too. Am
> I missing something?
>
> I hope I'm clear enough,
>
> Regards,
> --
> Alexandre Janniaux
> Videolabs
>
> On Mon, Jan 20, 2020 at 03:29:43PM +0200, Rémi Denis-Courmont wrote:
> > On second consideration, chopping the path is not correct behaviour. This patch is concealing the bug highlighted by the warning rather than fixing it IMO.
> >
> > Better drop the feature altogether as I dot think anyone's using it.
> >
> > Le 20 janvier 2020 09:38:46 GMT+02:00, "Rémi Denis-Courmont" <remi at remlab.net> a écrit :
> > >OK, though I'd suggest removing Unix socket support entirely as the old
> > >RC "protocol" is not suitable for programmatic usage.
> > >
> > >Le 19 janvier 2020 17:39:27 GMT+02:00, Alexandre Janniaux
> > ><ajanni at videolabs.io> a écrit :
> > >>Fortify enabled implies a warning if we use strncpy with a length
> > >equal
> > >>to the buffer size. As we fill the last item with a null character, we
> > >>can use length-1 instead as well.
> > >>---
> > >> modules/control/rc.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >>diff --git a/modules/control/rc.c b/modules/control/rc.c
> > >>index cd85602d59..dc37d352ad 100644
> > >>--- a/modules/control/rc.c
> > >>+++ b/modules/control/rc.c
> > >>@@ -1730,7 +1730,7 @@ static int Activate( vlc_object_t *p_this )
> > >>         }
> > >>
> > >>         addr.sun_family = AF_LOCAL;
> > >>-        strncpy( addr.sun_path, psz_unix_path, sizeof( addr.sun_path
> > >)
> > >>);
> > >>+        strncpy( addr.sun_path, psz_unix_path, sizeof( addr.sun_path
> > >)
> > >>- 1 );
> > >>         addr.sun_path[sizeof( addr.sun_path ) - 1] = '\0';
> > >>
> > >>         if (bind (i_socket, (struct sockaddr *)&addr, sizeof (addr))
> > >>--
> > >>2.25.0
> > >>
> > >>_______________________________________________
> > >>vlc-devel mailing list
> > >>To unsubscribe or modify your subscription options:
> > >>https://mailman.videolan.org/listinfo/vlc-devel
> > >
> > >--
> > >Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser
> > >ma brièveté.
> >
> > --
> > Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
>
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel