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 <[email protected]> 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 <[email protected]> > --- > 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 > [email protected] > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > -- Jonathan Rajotte Julien
_______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
