Argument injection in less-osc8-open enables RCE on link activation

[mat-mo]

The shipped script less-osc8-open.sh is registered as the default value of LESS_OSC8_OPEN_ANY in decode.c:

static char dflt_vartable[] =
{
    "LESS_OSC8_OPEN_ANY\0\201"
#ifdef LIBEXECDIR
        "-" LIBEXECDIR "/less-osc8-open"
#else
        "-less-osc8-open"
#endif
        "\0"
};

When the user activates an OSC 8 hyperlink (Ctrl-O Ctrl-O, or a second mouse click in mouse mode), osc8_open() in search.c invokes the handler with the URI as a single shell-quoted argument:

uri_q = shell_quoten(op.uri_start, uri_len);
cmd = ecalloc(strlen(handler) + strlen(uri_q) + 2, sizeof(char));
sprintf(cmd, "%s %s", handler, uri_q);
...
lsystem(exec_cmd, done_msg);

That delivers the URI verbatim into the script as $1. Inside the script, the man:NAME(SECTION) case splits the URI:

case $1 in
man:?*\(*\) )
    sect=${1#*\(}; sect=${sect%?}
    name=${1#man:};   name=${name%%\(*}
    man ${sect:+"$sect"} "$name"
    ;;

${sect:+"$sect"} expands to a single argv word, but man still interprets a word beginning with - as an option. Both GNU man-db and BSD man accept -P pager/-Ppager/--pager=pager with shell-tokenised pager strings, and man-db documents that the value "may be a simple command name or a command with arguments." Tokenisation alone is sufficient for execution: the pager argv is execvp'd with man's formatted output on stdin.

Reproduction

URI inside OSC 8 sequence:    man:ls(-P /usr/bin/touch /tmp/pwn)
OSC 8 wire form:              \x1b]8;;man:ls(-P /usr/bin/touch /tmp/pwn)\x1b\\click here\x1b]8;;\x1b\\

[avih]

Well, I wouldn't call it argument injection, but it is correct that it doesn't try to ensure that arguments are not interpreted as options by man, and it is true that some options may result in executing something - which was part of the OSC8 link.

Good catch.

Since man is a POSIX utility which does support the standard syntax guidelines, it should be supporting the -- options-operands separator, and indeed does support it at least on Linux.

If man supports -- pretty much everywhere (which it should, but I did not try to test), then we should simply insert -- - just like we already do with the invocation of less for file:// (because we do know that "less" does support --).

That would be the most robust solution IMO.

However, the form which we support with two arguments is apparently not a POSIX form. POSIX has this:

man [-k] name...
The man utility shall write information about each of the name operands. ...

There's no "The first operand is a section, and the second one is a name".

This makes me think about two things:

1. Eventhough less-osc8-open.sh does support a name without a section, groff never generates it as OSC8 links, but according to POSIX name without section is the only valid form. I think, at the very least, it should support a single name without section as OSC8 link target (not covering multiple names), but currently it doesn't generate such OSC8 links, because CC @g-branden-robinson :

To make a man page cross reference you really need to know the name. The man(1) librarian decides its own sectioning search order, so a page author can't rely upon the rendering system to find the intended page if they leave it (the section - avih) out.

2. Maybe we can't really know whether some versions of man get confused by an initial -- argument.

If we don't trust every version of man to support --, then we could do one of at least two things, both of which are trivial:

• Ensure none of the arguments begins with ASCII - (dash/minus).
• Or ensure the section (if it exist) always begins with ASCII digit, and name always beging with ASCII letter.

However, I'm not fond of these solutions as much as --, becasue maybe some options parsers accept non-dash as an option (like em dash, etc), and I don't know for a fact that all sections begin with ASCII digits and all names with ASCII letter (I never saw otherwise, but I don't know that).

Thoughts?

[mat-mo]

I opened PR #780 with that intention you suggested, but it might be a bit naive. I didn't test it across all man implementations, so it might not be robust enough.

[avih]

Both GNU man-db and BSD man accept -P pager/-Ppager/--pager=pager

Not exactly.

man-db is not gnu as far as I can tell, or at least its URL appears to be https://man-db.nongnu.org/ .

FreeBSD does support -P pager.

But as far as I can tell OpenBSD and NetBSD don't support pager as option. OmniOS (illumos - continuation of Open Solaris) also doesn't support a pager option as far as I can tell, and neither does Alpine linux.

Busybox man also doesn't support pager as option.

So pager as option is not ubiquitous, but it is indeed accepted in some linux distros and FreeBSD.

All the tested OSs/distros (Debian/Alpine Linux, Net/Free/Open BSD, OmniOS, busybox man, minix) do support -- correctly, i.e. ignore it if it precedes name and possibly section, and don't consider anything which follows as option, e.g. (OpenBSD):

# man -Z
man: unknown option -- Z
# man -- -Z
man: No entry for -Z in the manual.
#

So I think simply using man -- ... would be both ubiquitous enough, and would also make it an unattractive vector even if there exist some implementations of man which don't identify --.

@gwsw thoughts? It should probably be included in a fixup release, right?

[avih]

Somewhat related, as for avoiding the non-POSIX form man SECT NAME, there's no ubiquitous form to specify a section using POSIX notation as far as I can tell.

While POSIX allows an implementation to take more options, not all of them take the same options.

Some implementations accept -s SECT NAME (lowercase s), some accept -S SECT NAME (uppercase), some accept NAME.SECT (like printf.1).

But all of them accept the non-POSIX form which "less" uses - man -- SECT NAME (and the POSIX man -- NAME).

Somewhat annoying, but still ubiquitous enough.

[gwsw]

Yes, I think this should go into a bugfix release soon. It's unfortunate that there isn't a clear solution that's portable everywhere, but adding -- seems like the best solution. One might argue that if there's a system where man doesn't support -- then it should be the responsibility of the system integrator to modify less-osc8-open to work correctly with their man program, although realistically we can't rely on that.

[avih]

It's unfortunate that there isn't a clear solution that's portable everywhere

Well, all implementations which I could put my hands on do support man -- SECT NAME and man -- NAME, and POSIX does mandate that the -- is identified as such by man.

The only non-ubiquitous thing is how to specify a section, and the only place which disagrees with the common practice is POSIX itself...

But since man goes back such a long way, we can't know whether some older implementations which don't support the POSIX syntax guidelines are still in use.

Still, simply adding -- is both the least intrusive change, and likely also the best measure against such vectors, which, as noted, IMO does make it a very unattractive vector because in practice this measure prevents this vector everywhere meaningful.

[avih]

Well, all implementations which I could put my hands on do support man -- SECT NAME ...

To be clear - support it with the same semantics which less-osc8-open.sh intends.

All the mentioned alternative section syntaxes are in addition to this semantics. Not instead of it.

[gwsw]

Fixed in 41e7392.

[avih]

LGTM.

[g-branden-robinson]

There's no "The first operand is a section, and the second one is a name".

Not universally, no. This is an unfortunate limitation of POSIX. groff's Texinfo manual makes a similar observation.

https://www.gnu.org/software/groff/manual/groff.html.node/groff.html_fot.html#FOOT6

That said, Colin Watson's man-db supports fully three syntaxes:

• man 3 printf # "BSD"
• man -s 3 printf # "USG/System V"
• man 'printf(3)'

The last, while obviously demanding quotation for protection from an interactive shell or program running sh -c, has other strong advantages, like ease of delivery from other programs that should be using execve(2) instead of system(3) anyway.

Ingo Schwarze's mandoc supports the first two but not the last. mandoc owns the man page world on NetBSD and OpenBSD.

That leaves FreeBSD/macOS's shell script version of man(1), and the slowly dying lineage of System V. The latter works in the USG way noted above. I just checked a Solaris 11 system.

$ man -s 3c printf

I didn't bother studying FreeBSD's lengthy shell script, but according to its apparently comprehensive man page, it supports the following two modes of operation.

• man 3 printf
• man -S 3 printf

Note the capital S.

This makes me think about two things:

1. Eventhough `less-osc8-open.sh` does support a name without a section, groff never generates it as OSC8 links, but according to POSIX name without section is the only valid form. I think, at the very least, it should support a single name without section as OSC8 link target (not covering multiple names), but currently it doesn't generate such OSC8 links, [because](https://github.com/gwsw/less/pull/770#issuecomment-4432117281) CC [@g-branden-robinson](https://github.com/g-branden-robinson) :

I feel at the mercy of disagreeing implementations here. That, plus Apple's history of disagreeing with literally everyone else about how to represent hyperlinks to a man page, is what led me to indirect the link construction procedure in groff's man(7) package.

https://cgit.git.savannah.gnu.org/cgit/groff.git/tree/tmac/an.tmac?h=1.24.1#n1330

Until POSIX successfully imposes order here, I think the most I can do is let system administrators know how to ensure that man page hyperlinks are constructed in a way that the system is prepared for. That a system's man librarian also needs to be prepared for information rich enough to support a section indicator is a related, but separate, problem.

To make a man page cross reference you really need to know the name. The man(1) librarian decides its own sectioning search order, so a page author can't rely upon the rendering system to find the intended page if they leave it (the section - avih) out.

2. Maybe we can't really know whether some versions of `man` get confused by an initial `--` argument.

We can by experiment.

On Solaris 11, and I presume other surviving Unix System V installations:

-bash-4.4$ man -s 3c -- printf
Reformatting page.  Please Wait... done # worked as expected
-bash-4.4$ man -- printf
Reformatting page.  Please Wait... done # I got printf(1).
-bash-4.4$ man -- 'printf(3c)'
No manual entry for printf(3c). # failed as expected

With mandoc man(1), all of the following bring up the printf(3) document as I expect.

$ mman -- 3 printf
$ mman 3 -- printf
$ mman -s 3 -- printf

With man-db man(1), I get the same results.

I'm sure I haven't completely followed the thread here, but my conclusion is that using -- does not aid the man page resolution process in any way (barring man page names [or sections(!)] starting with a dash), but if it has advantages, employing it seems to have no drawbacks per the above experiments.

Some brave soul might want to attempt taking a run at the FreeBSD folks to get their man(1) to work more like that of man-db and mandoc. Maybe then POSIX will listen.

[avih]

I'm sure I haven't completely followed the thread here, but my conclusion is that using -- does not aid the man page resolution process in any way

I needs not aid resolution. It needs to solve the issue of arguments being interpreted as options by man.

Empiric observations with various implementations of man were summarized here #779 (comment) and in the following comment, and with all the tested implementations, a solution of -- to the issue at hand does address it correctly.

The reason I CC'ed you is in case you missed that in POSIX there's no way to specify a section, so you might want to support it in groff as well (properly - with OSC8 link). If you actively don't want to support it despite it being the only valid POSIX form/semantics, sure. I can't tell you what to do.

[g-branden-robinson]

I'm sure I haven't completely followed the thread here

Empiric observations with various implementations of man were summarized here #779 (comment) and in the following comment, and with all the tested implementations, a solution of -- to the issue at hand does address it correctly.

The reason I CC'ed you is in case you missed that in POSIX there's no way to specify a section,

No, I've been aware of it at least since...let's see...August 2023.

so you might want to support it in groff as well (properly - with OSC8 link).

I think you misunderstand what grotty's support for OSC 8 trying to do. It is trying to encode a URL. It is emphatically not attempting to construct a shell command for the user's host to run. I don't want to be in that business for the exact reason raised in this RCE report.

If you actively don't want to support it despite it being the only valid POSIX form/semantics, sure. I can't tell you what to do.

I think you have confused POSIX shell command syntax with RFC 3986(-esque) URI/URL syntax.

EDIT: Also, it occurs to me that you may not be aware that groff, and therefore I, am not responsible for supplying the man(1) command that runs on a system. That is why I spoke of man-db, mandoc, FreeBSD, and Solaris above. As I think I said more than once in #770, groff is the formatter for man pages. It is not the librarian that locates the document to be formatted. That's man(1)'s job. This separation of responsibilities goes back to the earliest versions of Unix in the 1970s.