[lttng-dev] [lttng-tools] Fix: channel name with '.' and '/' brings problems

[Jonathan Rajotte]

Hey Phil,

Not sure if modifying user input without any warnings about it is a good
idea. This clearly solve problems but do we want to solve it this way ?

It might be a better idea to warn the user about improper channel name or
simply block the command and return an error.

On Wed, Nov 26, 2014 at 10:32 PM, Philippe Proulx <eeppeliteloop at gmail.com>
wrote:

> This patch ensures:
>
>   1. A channel name does not contain any '/' character, since
>      relative paths may be injected in the channel name
>      otherwise (knowing that the channel name is eventually
>      part of a file name)
>   2. A channel name does not start with a '.' character, since
>      trace readers (Babeltrace is one of them) could interpret
>      files starting with a dot as hidden files and ignore
>      them when opening the CTF trace
>
> Fixes: #751
>
> Signed-off-by: Philippe Proulx <eeppeliteloop at gmail.com>
> ---
>  src/bin/lttng/commands/enable_channels.c | 38
> +++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/src/bin/lttng/commands/enable_channels.c
> b/src/bin/lttng/commands/enable_channels.c
> index f8272e9..e6cce49 100644
> --- a/src/bin/lttng/commands/enable_channels.c
> +++ b/src/bin/lttng/commands/enable_channels.c
> @@ -275,11 +275,39 @@ static int enable_channel(char *session_name)
>         /* Strip channel list (format: chan1,chan2,...) */
>         channel_name = strtok(opt_channels, ",");
>         while (channel_name != NULL) {
> -               /* Copy channel name and normalize it */
> +               /* Copy channel name, sanitize and normalize it */
>                 strncpy(chan.name, channel_name, NAME_MAX);
>                 chan.name[NAME_MAX - 1] = '\0';
>
> -               DBG("Enabling channel %s", channel_name);
> +               char *src, *dst;
> +               int got_first = 0;
> +
> +               for (src = dst = chan.name; *src != '\0'; ++src) {
> +                       *dst = *src;
> +
> +                       /*
> +                        * Channel name could be used in file names, so
> remove
> +                        * invalid '/'
> +                        */
> +                       if (*dst != '/') {
>

Maybe send some warning here ?


> +                               /*
> +                                * Remove starting dots since this could
> create
> +                                * file names starting with dots, and trace
> +                                * readers could interpret them as hidden
> files
> +                                * and ignore them.
> +                                */
> +                               if (*dst != '.') {
>

Same


> +                                       got_first = 1;
> +                                       dst++;
> +                               } else if (got_first) {
> +                                       dst++;
> +                               }
> +                       }
> +               }
> +
> +               *dst = '\0';
> +
> +               DBG("Enabling channel %s", chan.name);
>
>                 ret = lttng_enable_channel(handle, &chan);
>                 if (ret < 0) {
> @@ -288,19 +316,19 @@ static int enable_channel(char *session_name)
>                         case LTTNG_ERR_KERN_CHAN_EXIST:
>                         case LTTNG_ERR_UST_CHAN_EXIST:
>                         case LTTNG_ERR_CHAN_EXIST:
> -                               WARN("Channel %s: %s (session %s)",
> channel_name,
> +                               WARN("Channel %s: %s (session %s)",
> chan.name,
>                                                 lttng_strerror(ret),
> session_name);
>                                 warn = 1;
>                                 break;
>                         default:
> -                               ERR("Channel %s: %s (session %s)",
> channel_name,
> +                               ERR("Channel %s: %s (session %s)",
> chan.name,
>                                                 lttng_strerror(ret),
> session_name);
>                                 error = 1;
>                                 break;
>                         }
>                 } else {
>                         MSG("%s channel %s enabled for session %s",
> -                                       get_domain_str(dom.type),
> channel_name, session_name);
> +                                       get_domain_str(dom.type),
> chan.name, session_name);
>                         success = 1;
>                 }
>
> --
> 2.1.3
>
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>



-- 
Jonathan Rajotte Julien
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20141126/1a0a5bdf/attachment.html>