From 1c464c2da14d1d334a02bc94e65cec870bbf668a Mon Sep 17 00:00:00 2001 From: Grant Limberg Date: Fri, 15 Apr 2022 09:15:44 -0700 Subject: [PATCH] fix potential cstring leaks --- service/OneService.cpp | 26 +++++++++++++++++-------- zeroidc/src/ext.rs | 43 +++++++++++++++++++++++++++++++----------- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/service/OneService.cpp b/service/OneService.cpp index e585a464e..a014b0012 100644 --- a/service/OneService.cpp +++ b/service/OneService.cpp @@ -328,9 +328,10 @@ public: _config.ssoNonce ); - const char* url = zeroidc::zeroidc_get_auth_url(_idc); + char* url = zeroidc::zeroidc_get_auth_url(_idc); memcpy(_config.authenticationURL, url, strlen(url)); _config.authenticationURL[strlen(url)] = 0; + zeroidc::free_cstr(url); if (zeroidc::zeroidc_is_running(_idc) && nwc->status == ZT_NETWORK_STATUS_AUTHENTICATION_REQUIRED) { // TODO: kick the refresh thread @@ -362,23 +363,25 @@ public: return ""; } - const char* doTokenExchange(const char *code) { + char* doTokenExchange(const char *code) { #if ZT_SSO_ENABLED if (_idc == nullptr) { fprintf(stderr, "ainfo or idc null\n"); return ""; } - const char *ret = zeroidc::zeroidc_token_exchange(_idc, code); + char *ret = zeroidc::zeroidc_token_exchange(_idc, code); zeroidc::zeroidc_set_nonce_and_csrf( _idc, _config.ssoState, _config.ssoNonce ); - const char* url = zeroidc::zeroidc_get_auth_url(_idc); + char* url = zeroidc::zeroidc_get_auth_url(_idc); memcpy(_config.authenticationURL, url, strlen(url)); _config.authenticationURL[strlen(url)] = 0; + zeroidc::free_cstr(url); + return ret; #else return ""; @@ -1710,19 +1713,26 @@ public: } // SSO redirect handling - const char* state = zeroidc::zeroidc_get_url_param_value("state", path.c_str()); - const char* nwid = zeroidc::zeroidc_network_id_from_state(state); + char* state = zeroidc::zeroidc_get_url_param_value("state", path.c_str()); + char* nwid = zeroidc::zeroidc_network_id_from_state(state); const uint64_t id = Utils::hexStrToU64(nwid); + + zeroidc::free_cstr(nwid); + zeroidc::free_cstr(state); + Mutex::Lock l(_nets_m); if (_nets.find(id) != _nets.end()) { NetworkState& ns = _nets[id]; - const char* code = zeroidc::zeroidc_get_url_param_value("code", path.c_str()); - ns.doTokenExchange(code); + char* code = zeroidc::zeroidc_get_url_param_value("code", path.c_str()); + char *ret = ns.doTokenExchange(code); scode = 200; sprintf(resBuf, ssoResponseTemplate, "Authentication Successful. You may now access the network."); responseBody = std::string(resBuf); + zeroidc::free_cstr(code); + zeroidc::free_cstr(ret); + responseContentType = "text/html"; return scode; } else { diff --git a/zeroidc/src/ext.rs b/zeroidc/src/ext.rs index cb3afb88b..05752ab42 100644 --- a/zeroidc/src/ext.rs +++ b/zeroidc/src/ext.rs @@ -201,7 +201,28 @@ pub extern "C" fn zeroidc_set_nonce_and_csrf( ) )] #[no_mangle] -pub extern "C" fn zeroidc_get_auth_url(ptr: *mut ZeroIDC) -> *const c_char { +pub extern "C" fn free_cstr(s: *mut c_char) { + if s.is_null() { + println!("passed a null object"); + return; + } + + unsafe { + let _ = CString::from_raw(s); + } +} + +#[cfg( + any( + all(target_os = "linux", target_arch = "x86"), + all(target_os = "linux", target_arch = "x86_64"), + all(target_os = "linux", target_arch = "aarch64"), + target_os = "windows", + target_os = "macos", + ) +)] +#[no_mangle] +pub extern "C" fn zeroidc_get_auth_url(ptr: *mut ZeroIDC) -> *mut c_char { if ptr.is_null() { println!("passed a null object"); return std::ptr::null_mut(); @@ -224,15 +245,15 @@ pub extern "C" fn zeroidc_get_auth_url(ptr: *mut ZeroIDC) -> *const c_char { ) )] #[no_mangle] -pub extern "C" fn zeroidc_token_exchange(idc: *mut ZeroIDC, code: *const c_char ) -> *const c_char { +pub extern "C" fn zeroidc_token_exchange(idc: *mut ZeroIDC, code: *const c_char ) -> *mut c_char { if idc.is_null() { println!("idc is null"); - return std::ptr::null(); + return std::ptr::null_mut(); } if code.is_null() { println!("code is null"); - return std::ptr::null(); + return std::ptr::null_mut(); } let idc = unsafe { &mut *idc @@ -246,14 +267,14 @@ pub extern "C" fn zeroidc_token_exchange(idc: *mut ZeroIDC, code: *const c_char } #[no_mangle] -pub extern "C" fn zeroidc_get_url_param_value(param: *const c_char, path: *const c_char) -> *const c_char { +pub extern "C" fn zeroidc_get_url_param_value(param: *const c_char, path: *const c_char) -> *mut c_char { if param.is_null() { println!("param is null"); - return std::ptr::null(); + return std::ptr::null_mut(); } if path.is_null() { println!("path is null"); - return std::ptr::null(); + return std::ptr::null_mut(); } let param = unsafe {CStr::from_ptr(param)}.to_str().unwrap(); let path = unsafe {CStr::from_ptr(path)}.to_str().unwrap(); @@ -269,14 +290,14 @@ pub extern "C" fn zeroidc_get_url_param_value(param: *const c_char, path: *const } } - return std::ptr::null(); + return std::ptr::null_mut(); } #[no_mangle] -pub extern "C" fn zeroidc_network_id_from_state(state: *const c_char) -> *const c_char { +pub extern "C" fn zeroidc_network_id_from_state(state: *const c_char) -> *mut c_char { if state.is_null() { println!("state is null"); - return std::ptr::null(); + return std::ptr::null_mut(); } let state = unsafe{CStr::from_ptr(state)}.to_str().unwrap(); @@ -284,7 +305,7 @@ pub extern "C" fn zeroidc_network_id_from_state(state: *const c_char) -> *const let split = state.split("_"); let split = split.collect::>(); if split.len() != 2 { - return std::ptr::null(); + return std::ptr::null_mut(); } let s = CString::new(split[1]).unwrap();