Skip to content
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

[Security] Problem found in htop (your patches) #2777

Closed
BenBE opened this issue Apr 16, 2021 · 3 comments · Fixed by #2778
Closed

[Security] Problem found in htop (your patches) #2777

BenBE opened this issue Apr 16, 2021 · 3 comments · Fixed by #2778
Labels
Discussion Being discussed - Voice your opinions :)

Comments

@BenBE
Copy link

BenBE commented Apr 16, 2021

Describe the bug
Several security issues in your downstream patches to htop.

To reproduce
Have a look at those patches and see how many instances of unchecked buffer writes they contain, potentially allowing for DoS or code execution. One such major offender is the avafinger-cpu-monitor.patch. Look for strcpy.

Provide logs:
Please contact upstream for full details.

@lanefu
Copy link
Member

lanefu commented Apr 16, 2021

Do you have any suggested remediations?

@BenBE
Copy link
Author

BenBE commented Apr 16, 2021

In the easiest case you should replace those strcpy calls by checked calls like strncpy to ensure no write beyond the end of the allocated buffers is possible. Recent versions of htop also include a function String_safeStrncpy in XUtils.c, that further avoids one pitfal regarding plain use of strncpy alone.

Overall all externally provided strings should either be copied into dynamically managed buffers (xStrdup) or length-checked before further processing, when statically allocated buffers are used.

@piter75
Copy link
Member

piter75 commented Apr 16, 2021

@BenBE thanks for the heads-up!
The patches were copied over without proper attention :(
We will most probably borrow String_safeStrncpy from the current htop version to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Being discussed - Voice your opinions :)
Development

Successfully merging a pull request may close this issue.

4 participants