Stack-based Buffer Overflow in gwsw/less

Valid

Reported on

Sep 16th 2021


Description

The less utility is a pager used by many applications and setups. One such setup is access to log files. If permissions are not sufficient for regular users, less can be called with sudo. LESSSECURE=1 can be set to disable many dangerous operations which a regular user should not be able to run while having root privileges. Also less can be compiled with many features disabled to further reduce the possible attack vector.

Using less, sudo, and LESSSECURE=1 in such a setup is also described in the book "Sudo Mastery: 2nd Edition" by Michael W. Lucas.

Due to a formatting bug in opt_x() a regular user can override stack memory, including the return address of the current function. This in turn could allow code execution through return oriented programming, but it is very difficult to achieve because an attacker has only a very limited set of bytes to choose from.

Proof of Concept

Tested on Arch Linux with less 590. Any system with a running less is fine.

export LESSSECURE=1 # optional
less -x $(for i in $(seq 1000000000 1000000064); do echo -n "$i,"; done) -f /dev/null

Enter _x in less

*** stack smashing detected ***: terminated
Aborted (core dumped)

You can argue now that nobody would allow such -x arguments in a sudoers file, and you are correct! I added this so it is easier to copy&paste and reproduce. While less is running you can also simply enter "-x" and 64 integers manually. And that is what the attack is about.

Integers must be sorted in ascending order and can start with 1 and go up to INT_MAX, but no more than 64 integers are supported. Sounds like quite some flexibility but it will still be challenging to perform return oriented programming with this.

Impact

In best case the program simply crashes. In worst case an attacker is able to modify the program flow in a meaningful way and further instruct less to open other files or spawn processes.

Occurences

The msg array is not sufficient in size. Formatted integers can be longer than 4 bytes. 60+(11*TABSTOP_MAX) should be sufficient (11 includes a theoretical - character).

References

We created a GitHub Issue asking the maintainers to create a SECURITY.md 2 months ago
Jamie Slome
2 months ago

Admin


@stoeckmann - we saw that you bumped into a bug when trying to edit your report. This should now be resolved!

Apologies! 👋

Tobias
2 months ago

Researcher


Haha, thank you for your feedback and kudos to your own bug monitoring! I thought that twitter links would simply be not allowed. :)

Tobias Stoeckmann modified their report
2 months ago
Ziding Zhang
2 months ago

Admin


Hey Tobias, I've emailed the maintainers for you.

We have contacted a member of the gwsw/less team and are waiting to hear back 2 months ago
gwsw validated this vulnerability 2 months ago
Tobias Stoeckmann has been awarded the disclosure bounty
The fix bounty is now up for grabs
gwsw
2 months ago

Maintainer


This has been fixed in commit 6a860ee977eea7bfa065789ea4319ecab5af703c at https://github.com/gwsw/less.

Jamie Slome
2 months ago

Admin


@maintainer - are you able to confirm the fix with the button above? You will be able to select the branch and commit SHA that addresses the vulnerability.

gwsw confirmed that a fix has been merged on 6a860e 2 months ago
The fix bounty has been dropped
optfunc.c#L726 has been validated