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

`MODE #foo +o :user` gets parsed as `+o :user`, not `+o user` #601

Closed
dequis opened this Issue Jan 5, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@dequis
Member

dequis commented Jan 5, 2017

See sorcix/irc#26 for details (first part of the ticket has irssi-specific stuff)

Placing a breakpoint on modes_type_prefix() and looking at the backtrace, I see:

#0  modes_type_prefix (channel=0x55e8a29a8e20, setby=0x55e8a29a8bc1 "baz", type=43 '+', mode=111 'o',
    arg=0x55e8a29ae963 ":bar", newmode=0x55e8a2857200) at modes.c:301
#1  0x000055e8a0e137dd in parse_channel_modes (channel=0x55e8a29a8e20, setby=setby@entry=0x55e8a29a8bc1 "baz",
    mode=mode@entry=[…] "+o :bar", update_key=update_key@entry=1) at modes.c:372
#2  0x000055e8a0e13f1d in event_mode (server=0x55e8a29a6880, data=<optimized out>, nick=0x55e8a29a8bc1 "baz")
    at modes.c:498
#3  0x000055e8a0e3de92 in signal_emit_real (rec=rec@entry=0x55e8a286d690, params=params@entry=4, va=va@entry=0x7fffafbae100,
    first_hook=<optimized out>) at signals.c:242

AFAICT, irssi’s parse_channel_modes() function just doesn’t expect any of the parameters to contain the prefix (:) character.

event_mode:

	params = event_get_params(data, 2 | PARAM_FLAG_GETREST,
				  &channel, &mode);

event_get_params:

	rest = count & PARAM_FLAG_GETREST;
	count = PARAM_WITHOUT_FLAGS(count);

	while (count-- > 0) {
		str = (char **) va_arg(args, char **);
		if (count == 0 && rest) {
			/* put the rest to last parameter */
			tmp = *datad == ':' ? datad+1 : datad;

So : is only removed if it's in MODE #foo :+o user, not in MODE #foo +o :user.

It's technically valid rfc1459 framing so we have to fix it.

@dequis dequis added the bug label Jan 5, 2017

horgh added a commit to horgh/irssi that referenced this issue Oct 8, 2017

Correctly strip : from <trailing> parameters in MODE commands
This is to fix irssi#601. The prior function used to extract the mode string
assumed that ":" could occur only in a particular spot. This lead to the
possibility that ":" could be treated as part of things like nicknames
or mode arguments, where it should have been stripped as part of
protocol escaping.

We now recognize ":" as beginning a <trailing> parameter in any
position. There are still cases where this parsing is arguably not RFC
correct, but Irssi's mode parsing expects the mode arguments as a single
string already, so collapsing parameters in this way is probably okay.

As part of this change, I add a program to test both the new function
and the old. In order to use it, you need to run autogen.sh again, and
then run "make check".

horgh added a commit to horgh/irssi that referenced this issue Oct 8, 2017

Correctly strip : from <trailing> parameters in MODE commands
This is to fix irssi#601. The prior function used to extract the mode string
assumed that ":" could occur only in a particular spot. This lead to the
possibility that ":" could be treated as part of things like nicknames
or mode arguments, where it should have been stripped as part of
protocol escaping.

We now recognize ":" as beginning a <trailing> parameter in any
position. There are still cases where this parsing is arguably not RFC
correct, but Irssi's mode parsing expects the mode arguments as a single
string already, so collapsing parameters in this way is probably okay.

As part of this change, I add a program to test both the new function
and the old. In order to use it, you need to run autogen.sh again, and
then run "make check".

horgh added a commit to horgh/irssi that referenced this issue Oct 8, 2017

Strip : from MODE commands when parsing for the frontend
This is related to irssi#601. The prior commit fixed the recognition of
MODEs, but only internally. This changes to use the new MODE parameter
parsing when parsing for display in the client.

horgh added a commit to horgh/irssi that referenced this issue Oct 22, 2017

Correctly strip : from <trailing> parameters
This is to fix irssi#601. The function used to extract the mode string
assumed that ":" could occur only in a particular spot. This lead to the
possibility that ":" could be treated as part of things like nicknames
or mode arguments, where it should have been stripped as part of
protocol escaping.

We now recognize ":" as beginning a <trailing> parameter in any position
and remove it.

horgh added a commit to horgh/irssi that referenced this issue Oct 22, 2017

Correctly strip : from <trailing> parameters
This is to fix irssi#601. The function used to extract the mode string
assumed that ":" could occur only in a particular spot. This lead to the
possibility that ":" could be treated as part of things like nicknames
or mode arguments, where it should have been stripped as part of
protocol escaping.

We now recognize ":" as beginning a <trailing> parameter in any position
and remove it.

horgh added a commit to horgh/irssi that referenced this issue Oct 25, 2017

Strip : from <trailing> parameters
This is to fix irssi#601. The function used to extract the mode string
assumed that ":" would only occur in a particular spot. This lead to the
possibility that ":" could be treated as part of things like nicknames
or mode arguments, where it should have been stripped as part of
protocol escaping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment