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

[Rémi Denis-Courmont]

Hi,

Truncating a file path silently is wrong. I thought that's obvious TBH.

Le 20 janvier 2020 17:22:18 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
>Unfortunately, I'm not sure I understand what problem you're talking
>about. Can you be more explicit about it?
>
>AFAIU in the current situation, there was no issues but only an unsafe
>usage of strncpy which was not harmful as it was protected by an ending
>'\0' character enforcement. I don't see what the warning is
>highlighting otherwise.
>
>Regards,
>--
>Alexandre Janniaux
>Videolabs
>
>On Mon, Jan 20, 2020 at 04:44:32PM +0200, Rémi Denis-Courmont wrote:
>> Hi,
>>
>> The point is that the warning points to a real problem with the
>implementation, which the patch does not fix. And as a general rule,
>I'm against removing warnings without fixing the real problem.
>>
>> Le 20 janvier 2020 15:51:03 GMT+02:00, Alexandre Janniaux
><ajanni at videolabs.io> a écrit :
>> >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
>>
>> --
>> 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

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200120/fc0c53a1/attachment.html>