Przeglądaj źródła

fix potential cstring leaks

Grant Limberg 3 lat temu
rodzic
commit
1c464c2da1
2 zmienionych plików z 50 dodań i 19 usunięć
  1. 18 8
      service/OneService.cpp
  2. 32 11
      zeroidc/src/ext.rs

+ 18 - 8
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 {

+ 32 - 11
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::<Vec<&str>>();
     if split.len() != 2 {
-        return std::ptr::null();
+        return std::ptr::null_mut();
     }
 
     let s = CString::new(split[1]).unwrap();