Skip to content

last: avoid out of bounds array access #2843

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

last: avoid out of bounds array access #2843

wants to merge 1 commit into from

Conversation

biubiuzy
Copy link
Contributor

@biubiuzy biubiuzy commented Mar 18, 2024 •
edited
Loading

In the fuzz test, the value of when variable may be very large(e.g 88123456123456123456),which can not generate a correct time string, may be cause array out-of-bounds access.

#include <stdio.h>
#include <time.h>

int main() {
        time_t big_time = 88123456123456123456;
        char buf[26];
        int ret = ctime_r(&big_time, buf);
        printf("ret: %d\nbuf:", ret);
        for(int i = 0; i < 26; i++)
        {
                printf("0x%02x ",buf[i]);
        }

        printf("\n");
}

ret: 0
buf:0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x60 0x58

@biubiuzy
Copy link
Contributor Author

==2088193==ERROR: AddressSanitizer: stack-buffer-overflow on address 0xffffffffbfda at pc 0x0000004ffa90 bp 0xffffffffacd0 sp 0xffffffffad78
READ of size 31 at 0xffffffffbfda thread T0
[Detaching after fork from child process 2088196]
#0 0x4ffa8c in printf_common(void*, char const*, std::__va_list) (/home/trial-1562.util-linux.test_last_fuzz/main-artifacts/test_last_fuzz+0x4ffa8c)
#1 0x506dd8 in vsnprintf (/home/trial-1562.util-linux.test_last_fuzz/main-artifacts/test_last_fuzz+0x506dd8)
#2 0x50708c in snprintf (/home/trial-1562.util-linux.test_last_fuzz/main-artifacts/test_last_fuzz+0x50708c)
#3 0x565418 in time_formatter /home/build-1561.util-linux.EulerOS.testblitz+asan/build-util-linux/src/util-linux/login-utils/last.c:364:3
#4 0x562ca0 in process_wtmp_file /home/build-1561.util-linux.EulerOS.testblitz+asan/build-util-linux/src/util-linux/login-utils/last.c:908:7
#5 0x56154c in LLVMFuzzerTestOneInput /home/build-1561.util-linux.EulerOS.testblitz+asan/build-util-linux/src/util-linux/login-utils/last.c:946:2
#6 0x588d74 in testblitz_engine::runtime::runner::run_raw::h0d8c7b154d90cbb3 /root/xTest/TestBlitz/testblitz-engine/src/runtime/runner.rs:86:14
#7 0x588d74 in testblitz_engine::runtime::runner::run::h2eeda5b4745d0534 /root/xTest/TestBlitz/testblitz-engine/src/runtime/runner.rs:93:5
#8 0x56df74 in testblitz_engine::fuzzer::Fuzzer::run_one::h652e2fe2f434d41d /root/xTest/TestBlitz/testblitz-engine/src/fuzzer/mod.rs:76:9
#9 0x5888a4 in testblitz_engine::fuzzer::Fuzzer::add_seeds::$u7b$$u7b$closure$u7d$$u7d$::$u7b$$u7b$closure$u7d$$u7d$::h9a7a9b7579fd0ac0 /root/xTest/TestBlitz/testblitz-engine/src/fuzzer/mod.rs:185:17
#10 0x5888a4 in tracing::span::Span::in_scope::hcc6e6ca2ba6f77ec /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.40/src/span.rs:1102:9
#11 0x5888a4 in testblitz_engine::fuzzer::Fuzzer::add_seeds::$u7b$$u7b$closure$u7d$$u7d$::h37afc402843cf4a4 /root/xTest/TestBlitz/testblitz-engine/src/fuzzer/mod.rs:183:13
#12 0x5888a4 in testblitz_engine::fuzzer::util::walkdir::hc060f005382e036a /root/xTest/TestBlitz/testblitz-engine/src/fuzzer/util.rs:80:28
#13 0x56cb44 in testblitz_engine::fuzzer::Fuzzer::add_seeds::h812504eb332daf62 /root/xTest/TestBlitz/testblitz-engine/src/fuzzer/mod.rs:181:9
#14 0x56cb44 in testblitz_engine::entrypoint::init_fuzzer::hfe70d191897e9d50 /root/xTest/TestBlitz/testblitz-engine/src/entrypoint.rs:97:9
#15 0x56cb44 in testblitz_engine::entrypoint::run_fuzz_loop::h707b34a422f7026d /root/xTest/TestBlitz/testblitz-engine/src/entrypoint.rs:117:17
#16 0x56cb44 in testblitz_engine::entrypoint::main::$u7b$$u7b$closure$u7d$$u7d$::h9e6620a3ed0e7bad /root/xTest/TestBlitz/testblitz-engine/src/entrypoint.rs:147:27
#17 0x56c0f0 in main /root/xTest/TestBlitz/testblitz-engine/src/entrypoint.rs
#18 0xfffff7b6403c (/usr/lib64/libc.so.6+0x2b03c)
#19 0xfffff7b64114 in __libc_start_main (/usr/lib64/libc.so.6+0x2b114)
#20 0x49bf6c in _start (/home/trial-1562.util-linux.test_last_fuzz/main-artifacts/test_last_fuzz+0x49bf6c)

Address 0xffffffffbfda is located in stack of thread T0 at offset 154 in frame
#0 0x5651e8 in time_formatter /home/build-1561.util-linux.EulerOS.testblitz+asan/build-util-linux/src/util-linux/login-utils/last.c:343

This frame has 2 object(s):
[32, 88) 'tm' (line 352)
[128, 154) 'buf' (line 361) <== Memory access at offset 154 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork

@t-8ch
Copy link
Member

t-8ch commented Mar 18, 2024

We should instead check the return value of ctime_r.
It is not an int but char *. If it is NULL then the call failed.

@biubiuzy
Copy link
Contributor Author

We should instead check the return value of ctime_r. It is not an int but char *. If it is NULL then the call failed.

Done！

@t-8ch
Copy link
Member

t-8ch commented Mar 18, 2024

Thanks. Looking at the whole of util-linux, it seems no return values of ctime_r() and nearly non of the return values of localtime_r() are checked.

@biubiuzy
Copy link
Contributor Author

Thanks. Looking at the whole of util-linux, it seems no return values of ctime_r() and nearly non of the return values of localtime_r() are checked.

Yeah. need to fix it in this PR?

@t-8ch
Copy link
Member

t-8ch commented Mar 19, 2024

need to fix it in this PR?

(I can't make any demands, I'm not the maintainer)

IMO it makes sense to have a PR taking care of all calls.

@karelzak
Copy link
Collaborator

Please use another PR for the other ctime_r() issues.

@karelzak
Copy link
Collaborator

Merged as: 75822ef
I did a local rebase before pushing (due to a change in another patch), so GitHub could not detect that this PR had already been merged.

@karelzak karelzak closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@biubiuzy @t-8ch @karelzak