From 14988e33b2e1a3428998cb24793fbe717f16add1 Mon Sep 17 00:00:00 2001 From: Josh Megnauth Date: Fri, 13 Dec 2024 01:20:24 -0500 Subject: [PATCH] Unsafe blocks: `getopt.h`, `grp.h` --- src/header/getopt/mod.rs | 133 ++++++++++++++++++++++----------------- src/header/grp/mod.rs | 67 ++++++++++++-------- 2 files changed, 116 insertions(+), 84 deletions(-) diff --git a/src/header/getopt/mod.rs b/src/header/getopt/mod.rs index d793ab8613..0c3f5d97fa 100644 --- a/src/header/getopt/mod.rs +++ b/src/header/getopt/mod.rs @@ -1,5 +1,7 @@ //! getopt implementation for relibc +#![deny(unsafe_op_in_unsafe_fn)] + use crate::{ header::{ stdio, string, @@ -33,94 +35,107 @@ pub unsafe extern "C" fn getopt_long( longindex: *mut c_int, ) -> c_int { // if optarg is not set, we still don't want the previous value leaking - optarg = ptr::null_mut(); - - // handle reinitialization request - if optind == 0 { - optind = 1; - CURRENT_OPT = ptr::null_mut(); + unsafe { + optarg = ptr::null_mut(); } - if CURRENT_OPT.is_null() || *CURRENT_OPT == 0 { - if optind >= argc { + // handle reinitialization request + unsafe { + if optind == 0 { + optind = 1; + CURRENT_OPT = ptr::null_mut(); + } + } + + if unsafe { CURRENT_OPT.is_null() || *CURRENT_OPT == 0 } { + if unsafe { optind >= argc } { -1 } else { - let current_arg = *argv.offset(optind as isize); - if current_arg.is_null() - || *current_arg != b'-' as c_char - || *current_arg.offset(1) == 0 - { + let current_arg = unsafe { *argv.offset(optind as isize) }; + if unsafe { + current_arg.is_null() + || *current_arg != b'-' as c_char + || *current_arg.offset(1) == 0 + } { -1 - } else if string::strcmp(current_arg, c_str!("--").as_ptr()) == 0 { - optind += 1; + } else if unsafe { string::strcmp(current_arg, c_str!("--").as_ptr()) == 0 } { + unsafe { + optind += 1; + } -1 } else { // remove the '-' - let current_arg = current_arg.offset(1); + let current_arg = unsafe { current_arg.offset(1) }; - if *current_arg == b'-' as c_char && !longopts.is_null() { - let current_arg = current_arg.offset(1); + if unsafe { *current_arg == b'-' as c_char } && !longopts.is_null() { + let current_arg = unsafe { current_arg.offset(1) }; // is a long option for i in 0.. { - let opt = &*longopts.offset(i); + let opt = unsafe { &*longopts.offset(i) }; if opt.name.is_null() { break; } let mut end = 0; while { - let c = *current_arg.offset(end); + let c = unsafe { *current_arg.offset(end) }; c != 0 && c != b'=' as c_char } { end += 1; } - if string::strncmp(current_arg, opt.name, end as size_t) == 0 { - optind += 1; - *longindex = i as c_int; + if unsafe { string::strncmp(current_arg, opt.name, end as size_t) == 0 } { + unsafe { + optind += 1; + *longindex = i as c_int; + } if opt.has_arg == optional_argument { - if *current_arg.offset(end) == b'=' as c_char { - optarg = current_arg.offset(end + 1); + unsafe { + if *current_arg.offset(end) == b'=' as c_char { + optarg = current_arg.offset(end + 1); + } } } else if opt.has_arg == required_argument { - if *current_arg.offset(end) == b'=' as c_char { - optarg = current_arg.offset(end + 1); - } else if optind < argc { - optarg = *argv.offset(optind as isize); - optind += 1; - } else if *optstring == b':' as c_char { - return b':' as c_int; - } else { - stdio::fputs(*argv as _, &mut *stdio::stderr); - stdio::fputs( - ": option '--\0".as_ptr() as _, - &mut *stdio::stderr, - ); - stdio::fputs(current_arg, &mut *stdio::stderr); - stdio::fputs( - "' requires an argument\n\0".as_ptr() as _, - &mut *stdio::stderr, - ); - return b'?' as c_int; + unsafe { + if *current_arg.offset(end) == b'=' as c_char { + optarg = current_arg.offset(end + 1); + } else if optind < argc { + optarg = *argv.offset(optind as isize); + optind += 1; + } else if *optstring == b':' as c_char { + return b':' as c_int; + } else { + stdio::fputs(*argv as _, &mut *stdio::stderr); + stdio::fputs( + ": option '--\0".as_ptr() as _, + &mut *stdio::stderr, + ); + stdio::fputs(current_arg, &mut *stdio::stderr); + stdio::fputs( + "' requires an argument\n\0".as_ptr() as _, + &mut *stdio::stderr, + ); + return b'?' as c_int; + } } } if opt.flag.is_null() { return opt.val; } else { - *opt.flag = opt.val; + unsafe { *opt.flag = opt.val }; return 0; } } } } - parse_arg(argc, argv, current_arg, optstring) + unsafe { parse_arg(argc, argv, current_arg, optstring) } } } } else { - parse_arg(argc, argv, CURRENT_OPT, optstring) + unsafe { parse_arg(argc, argv, CURRENT_OPT, optstring) } } } @@ -130,14 +145,14 @@ unsafe fn parse_arg( current_arg: *mut c_char, optstring: *const c_char, ) -> c_int { - let update_current_opt = || { + let update_current_opt = || unsafe { CURRENT_OPT = current_arg.offset(1); if *CURRENT_OPT == 0 { optind += 1; } }; - let print_error = |desc: &[u8]| { + let print_error = |desc: &[u8]| unsafe { // NOTE: we don't use fprintf to get around the usage of va_list stdio::fputs(*argv as _, &mut *stdio::stderr); stdio::fputs(desc.as_ptr() as _, &mut *stdio::stderr); @@ -145,13 +160,13 @@ unsafe fn parse_arg( stdio::fputc(b'\n' as _, &mut *stdio::stderr); }; - match find_option(*current_arg, optstring) { + match unsafe { find_option(*current_arg, optstring) } { Some(GetoptOption::Flag) => { update_current_opt(); - *current_arg as c_int + unsafe { *current_arg as c_int } } - Some(GetoptOption::OptArg) => { + Some(GetoptOption::OptArg) => unsafe { CURRENT_OPT = b"\0".as_ptr() as _; if *current_arg.offset(1) == 0 { optind += 2; @@ -180,16 +195,18 @@ unsafe fn parse_arg( *current_arg as c_int } - } + }, None => { // couldn't find the given option in optstring - if opterr != 0 { + if unsafe { opterr != 0 } { print_error(b": illegal option -- \0"); } update_current_opt(); - optopt = *current_arg as c_int; + unsafe { + optopt = *current_arg as c_int; + } b'?' as c_int } } @@ -203,9 +220,9 @@ enum GetoptOption { unsafe fn find_option(ch: c_char, optstring: *const c_char) -> Option { let mut i = 0; - while *optstring.offset(i) != 0 { - if *optstring.offset(i) == ch { - let result = if *optstring.offset(i + 1) == b':' as c_char { + while unsafe { *optstring.offset(i) != 0 } { + if unsafe { *optstring.offset(i) == ch } { + let result = if unsafe { *optstring.offset(i + 1) == b':' as c_char } { GetoptOption::OptArg } else { GetoptOption::Flag diff --git a/src/header/grp/mod.rs b/src/header/grp/mod.rs index 700db8df74..49af5b4160 100644 --- a/src/header/grp/mod.rs +++ b/src/header/grp/mod.rs @@ -1,5 +1,7 @@ //! grp implementation, following http://pubs.opengroup.org/onlinepubs/7908799/xsh/grp.h.html +#![deny(unsafe_op_in_unsafe_fn)] + use core::{ cell::SyncUnsafeCell, convert::{TryFrom, TryInto}, @@ -288,12 +290,13 @@ pub unsafe extern "C" fn getgrnam(name: *const c_char) -> *mut group { }; // Attempt to prevent BO vulnerabilities - if strncmp( - grp.reference.gr_name, - name, - strlen(grp.reference.gr_name).min(strlen(name)), - ) > 0 - { + if unsafe { + strncmp( + grp.reference.gr_name, + name, + strlen(grp.reference.gr_name).min(strlen(name)), + ) > 0 + } { return grp.into_global(); } } @@ -311,7 +314,9 @@ pub unsafe extern "C" fn getgrgid_r( result: *mut *mut group, ) -> c_int { // In case of error or the requested entry is not found. - *result = ptr::null_mut(); + unsafe { + *result = ptr::null_mut(); + } let Ok(db) = File::open(c_str!("/etc/group"), fcntl::O_RDONLY) else { return ENOENT; @@ -345,8 +350,10 @@ pub unsafe extern "C" fn getgrgid_r( }; if grp.reference.gr_gid == gid { - *result_buf = grp.reference; - *result = result_buf; + unsafe { + *result_buf = grp.reference; + *result = result_buf; + } return 0; } @@ -381,14 +388,17 @@ pub unsafe extern "C" fn getgrnam_r( return EINVAL; }; - if strncmp( - grp.reference.gr_name, - name, - strlen(grp.reference.gr_name).min(strlen(name)), - ) > 0 - { - *result_buf = grp.reference; - *result = result_buf; + if unsafe { + strncmp( + grp.reference.gr_name, + name, + strlen(grp.reference.gr_name).min(strlen(name)), + ) > 0 + } { + unsafe { + *result_buf = grp.reference; + *result = result_buf; + } return 0; } @@ -400,7 +410,7 @@ pub unsafe extern "C" fn getgrnam_r( // MT-Unsafe race:grent race:grentbuf locale #[no_mangle] pub unsafe extern "C" fn getgrent() -> *mut group { - let mut line_reader = &mut *LINE_READER.get(); + let mut line_reader = unsafe { &mut *LINE_READER.get() }; if line_reader.is_none() { let Ok(db) = File::open(c_str!("/etc/group"), fcntl::O_RDONLY) else { @@ -430,13 +440,15 @@ pub unsafe extern "C" fn getgrent() -> *mut group { // MT-Unsafe race:grent locale #[no_mangle] pub unsafe extern "C" fn endgrent() { - *(&mut *LINE_READER.get()) = None; + unsafe { + *(&mut *LINE_READER.get()) = None; + } } // MT-Unsafe race:grent locale #[no_mangle] pub unsafe extern "C" fn setgrent() { - let mut line_reader = &mut *LINE_READER.get(); + let mut line_reader = unsafe { &mut *LINE_READER.get() }; let Ok(db) = File::open(c_str!("/etc/group"), fcntl::O_RDONLY) else { return; }; @@ -452,13 +464,14 @@ pub unsafe extern "C" fn getgrouplist( groups: *mut gid_t, ngroups: *mut c_int, ) -> c_int { - let mut grps = - slice::from_raw_parts_mut(groups.cast::>(), ngroups.read() as usize); + let mut grps = unsafe { + slice::from_raw_parts_mut(groups.cast::>(), ngroups.read() as usize) + }; // FIXME: This API probably expects the group database to already exist in memory, as it // doesn't seem to have any documented error handling. - let Ok(user) = (crate::c_str::CStr::from_ptr(user).to_str()) else { + let Ok(user) = (unsafe { crate::c_str::CStr::from_ptr(user).to_str() }) else { return 0; }; @@ -499,7 +512,9 @@ pub unsafe extern "C" fn getgrouplist( }; } - ngroups.write(groups_found); + unsafe { + ngroups.write(groups_found); + } if groups_found as usize > grps.len() { -1 @@ -514,8 +529,8 @@ pub unsafe extern "C" fn getgrouplist( pub unsafe extern "C" fn initgroups(user: *const c_char, gid: gid_t) -> c_int { let mut groups = [0; limits::NGROUPS_MAX]; let mut count = groups.len() as c_int; - if getgrouplist(user, gid, groups.as_mut_ptr(), &mut count) < 0 { + if unsafe { getgrouplist(user, gid, groups.as_mut_ptr(), &mut count) < 0 } { return -1; } - unistd::setgroups(count as size_t, groups.as_ptr()) + unsafe { unistd::setgroups(count as size_t, groups.as_ptr()) } }