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

Android logging may need reencoding of the logged values or may be unsound #12728

Closed
james7132 opened this issue Mar 26, 2024 · 1 comment · Fixed by #12743
Closed

Android logging may need reencoding of the logged values or may be unsound #12728

james7132 opened this issue Mar 26, 2024 · 1 comment · Fixed by #12743
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Bug An unexpected or incorrect behavior O-Android Specific to the Android mobile operating system P-Unsound A bug that results in undefined compiler behavior
Milestone

Comments

@james7132
Copy link
Member

Found in #12684. The call into android_log_sys::__android_log_write may be unsound here:

let message = format!("{}\0", recorder);
let tag = format!("{}\0", meta.name());
unsafe {
android_log_sys::__android_log_write(
priority as android_log_sys::c_int,
tag.as_ptr() as *const android_log_sys::c_char,
message.as_ptr() as *const android_log_sys::c_char,
);

This unsafe call is undocumented and casts two UTF-8 encoded strings as if they were using the OS encoding. They're properly null-terminated, but probably should be converted into OsString or CString before passing into the function.

@james7132 james7132 added C-Bug An unexpected or incorrect behavior O-Android Specific to the Android mobile operating system A-Diagnostics Logging, crash handling, error reporting and performance analysis P-Unsound A bug that results in undefined compiler behavior labels Mar 26, 2024
@james7132 james7132 added this to the 0.14 milestone Mar 26, 2024
@BD103
Copy link
Member

BD103 commented Mar 26, 2024

Linking docs.rs and Android's Docs. I believe CString works best here.

github-merge-queue bot pushed a commit that referenced this issue Mar 29, 2024
# Objective
Fix #12728. Fix unsoundnesss from unhandled null characters in Android
logs.

## Solution
Use `CString` instead of using formatted Strings. Properly document the
safety invariants of the FFI call.
mockersf pushed a commit that referenced this issue Apr 1, 2024
# Objective
Fix #12728. Fix unsoundnesss from unhandled null characters in Android
logs.

## Solution
Use `CString` instead of using formatted Strings. Properly document the
safety invariants of the FFI call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Bug An unexpected or incorrect behavior O-Android Specific to the Android mobile operating system P-Unsound A bug that results in undefined compiler behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants