Skip to content

linux: fix out-of-bounds access when parsing power supply type - #2065

Merged
BenBE merged 1 commit into
htop-dev:mainfrom
lilu5458:fix-battery-type-oob
Aug 11, 2026
Merged

linux: fix out-of-bounds access when parsing power supply type#2065
BenBE merged 1 commit into
htop-dev:mainfrom
lilu5458:fix-battery-type-oob

Conversation

@lilu5458

Copy link
Copy Markdown
Contributor

Problem

Platform_Battery_getSysData() in linux/Platform.c reads /sys/class/power_supply/<name>/type into a stack buffer and trims trailing newlines with:

char buffer[32];
ssize_t ret = Compat_readfileat(entryFd, "type", buffer, sizeof(buffer));
if (ret <= 0)
   goto next;

/* drop optional trailing newlines */
for (char* buf = &buffer[(size_t)ret - 1]; *buf == '\n'; buf--)
   *buf = '\0';

Compat_readfileat() reads up to count - 1 bytes (reserving one for the '\0' terminator), so for a 32-byte buffer it can return up to 31 bytes. If the type file contains only newline bytes (e.g. 31 '\n'), the loop walks the pointer from buffer[30] down to buffer[0] and then — because there is no lower-bound check — performs an out-of-bounds read at buffer[-1] to re-evaluate *buf == '\n'.

The loop only stops there if the OOB byte is not '\n'. If the OOB byte happens to be 0x0a (e.g. leftover stack content), the loop continues:

  • writing '\0' into buffer[-1] (OOB write), and
  • reading buffer[-2], buffer[-3], …

corrupting the stack below the buffer for as many consecutive '\n' bytes as are present, until a non-'\n' byte or an unmapped page is reached.

Trigger

A malicious or buggy power_supply device whose type attribute contains only newlines. Such devices can be created via USB/ACPI/container power supply class registrations (e.g. /sys/class/power_supply/<name>/type). Any entry whose name does not start with BAT or AC reaches this code path.

Impact

  • OOB stack read is guaranteed whenever the type file is all newlines.
  • OOB stack write (and unbounded downward walk) occurs when the OOB bytes read are '\n'.

This is reachable from an unprivileged trigger (a crafted power_supply sysfs entry) and can corrupt the stack of the Platform_Battery_getSysData frame.

Fix

Add a lower-bound check so the loop stops at buffer[0]:

for (char* buf = &buffer[(size_t)ret - 1]; buf >= buffer && *buf == '\n'; buf--)
   *buf = '\0';

One line, one file.

Verification

Reproduced the OOB with a standalone canary test that mimics Compat_readfileat returning 31 newlines into a 32-byte buffer sandwiched between guard regions:

  • With the guard byte below buffer[0] set to a non-'\n' value, the loop performs an OOB read at buffer[-1] (final pointer offset -1) and stops only because that byte is not '\n'.
  • With 3 '\n' bytes placed below buffer[0], the loop performs 3 OOB writes, clobbering the guard bytes from 0x0a to 0x00, and walks to offset -4.

With this patch applied, the loop stops at buffer[0] and no OOB access occurs. htop rebuilds cleanly with -Wall -Wextra.

Notes

Assisted-by: lilu lilu@kylinos.cn

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Platform_Battery_getSysData now rejects reads larger than the buffer. It truncates the value at the first non-printable character instead of using an unbounded trailing-newline removal loop.

Possibly related PRs

Poem

Battery data enters the light,
Length checks keep each read right.
Non-printable marks the end,
Safe bounds guard the data’s trend.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ae79f4b9-e1b5-42b0-9c19-6cf53ee9adc6

📥 Commits

Reviewing files that changed from the base of the PR and between 443a414 and 82172e6.

📒 Files selected for processing (1)
  • linux/Platform.c

Comment thread linux/Platform.c Outdated
When reading /sys/class/power_supply/<name>/type, the trailing-newline
trimming loop in Platform_Battery_getSysData started at
&buffer[ret - 1] and decremented the pointer while *buf == '\n',
without checking that the pointer remained within the buffer.

If the type file contains only newline bytes, the loop walks past
buffer[0] and performs an out-of-bounds read on the stack. If the OOB
byte happens to be '\n' (0x0a), the loop also writes '\0' into it,
corrupting the stack below the buffer for as many consecutive newline
bytes as are present.

Run the loop forward instead and sanitize control characters and spaces.

Assisted-by: lilu <lilu@kylinos.cn>
Co-authored-by: BenBE <BenBE@geshi.org>
@BenBE BenBE self-assigned this Aug 11, 2026
@BenBE BenBE added this to the 3.5.3 milestone Aug 11, 2026
@BenBE BenBE added bug 🐛 Something isn't working Linux 🐧 Linux related issues security 👮 Issues with security implications labels Aug 11, 2026
@BenBE
BenBE force-pushed the fix-battery-type-oob branch from 82172e6 to 4329f54 Compare August 11, 2026 16:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 180af140-80c8-4a94-adae-6b3b96f126d6

📥 Commits

Reviewing files that changed from the base of the PR and between 82172e6 and 4329f54.

📒 Files selected for processing (1)
  • linux/Platform.c

Comment thread linux/Platform.c
@BenBE
BenBE merged commit ac6649c into htop-dev:main Aug 11, 2026
21 checks passed
@BenBE BenBE linked an issue Aug 14, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐛 Something isn't working Linux 🐧 Linux related issues security 👮 Issues with security implications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dumping core on startup

2 participants