From 5d71f95ef74203fc433729b4026c74c4606d2a2b Mon Sep 17 00:00:00 2001 From: kallal79 Date: Fri, 3 Apr 2026 01:04:47 +0530 Subject: [PATCH] Security fix #524: prevent path traversal, unsafe eval, and key leakage --- concore.hpp | 21 ++++++++++++++-- concore.py | 69 +++++++++++++++++++++++++++++++++++++++++++++------ demo/pwrap.py | 5 +++- ratc/pwrap.py | 5 +++- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/concore.hpp b/concore.hpp index b74ddd7e..3fec1081 100644 --- a/concore.hpp +++ b/concore.hpp @@ -41,6 +41,14 @@ class Concore{ int communication_iport = 0; // iport refers to input port int communication_oport = 0; // oport refers to input port + bool isSafeName(const std::string& name) { + if (name.empty()) return false; + if (name == "." || name == "..") return false; + if (name.find("..") != std::string::npos) return false; + if (name.find('/') != std::string::npos || name.find('\\') != std::string::npos) return false; + return true; + } + public: double delay = 1; int retrycount = 0; @@ -133,7 +141,7 @@ class Concore{ */ void createSharedMemory(key_t key) { - shmId_create = shmget(key, 256, IPC_CREAT | 0666); + shmId_create = shmget(key, 256, IPC_CREAT | 0600); if (shmId_create == -1) { std::cerr << "Failed to create shared memory segment." << std::endl; @@ -155,7 +163,7 @@ class Concore{ { while (true) { // Get the shared memory segment created by Writer - shmId_get = shmget(key, 256, 0666); + shmId_get = shmget(key, 256, 0600); // Check if shared memory exists if (shmId_get != -1) { break; // Break the loop if shared memory exists @@ -284,6 +292,9 @@ class Concore{ * @return a string of file content */ vector read_FM(int port, string name, string initstr){ + if (!isSafeName(name)) { + return vector(); + } chrono::milliseconds timespan((int)(1000*delay)); this_thread::sleep_for(timespan); string ins; @@ -440,6 +451,9 @@ class Concore{ * @param delta The delta value (default: 0). */ void write_FM(int port, string name, vector val, int delta=0){ + if (!isSafeName(name)) { + return; + } try { ofstream outfile; @@ -470,6 +484,9 @@ class Concore{ * @param delta The delta value (default: 0). */ void write_FM(int port, string name, string val, int delta=0){ + if (!isSafeName(name)) { + return; + } chrono::milliseconds timespan((int)(2000*delay)); this_thread::sleep_for(timespan); try { diff --git a/concore.py b/concore.py index 6d71f0fc..fc97a054 100644 --- a/concore.py +++ b/concore.py @@ -3,6 +3,7 @@ from ast import literal_eval import sys import re +import json import zmq # Added for ZeroMQ # if windows, create script to kill this process @@ -67,6 +68,52 @@ def recv_json_with_retry(self): # Global ZeroMQ ports registry zmq_ports = {} +def _normalize_zmq_address(port_type, address): + if not isinstance(address, str): + return address + addr = address.strip() + if port_type == "bind" and addr.startswith("tcp://*:"): + if os.environ.get("CONCORE_ALLOW_WILDCARD_BIND", "").lower() not in ("1", "true", "yes"): + safe_addr = "tcp://127.0.0.1:" + addr.split(":")[-1] + print(f"Warning: Rewriting insecure bind address '{addr}' to '{safe_addr}'.") + return safe_addr + return addr + +def _is_safe_name(name): + if not isinstance(name, str) or name == "": + return False + if os.path.basename(name) != name or "/" in name or "\\" in name: + return False + if name in (".", "..") or ".." in name: + return False + return True + +def _safe_channel_path(base_path_prefix, port_identifier, name): + if not _is_safe_name(name): + raise ValueError(f"Unsafe channel name: {name}") + port_str = str(int(port_identifier)) + channel_dir = os.path.abspath(base_path_prefix + port_str) + file_path = os.path.abspath(os.path.join(channel_dir, name)) + if os.path.commonpath([channel_dir, file_path]) != channel_dir: + raise ValueError(f"Path traversal detected for channel name: {name}") + return file_path + +def _parse_untrusted_value(raw_value, default_value): + if not isinstance(raw_value, str): + return raw_value + text = raw_value.strip() + if text == "": + return default_value + try: + return json.loads(text) + except Exception: + pass + try: + return literal_eval(text) + except Exception: + return default_value + + def init_zmq_port(port_name, port_type, address, socket_type_str): """ Initializes and registers a ZeroMQ port. @@ -82,8 +129,9 @@ def init_zmq_port(port_name, port_type, address, socket_type_str): try: # Map socket type string to actual ZMQ constant (e.g., zmq.REQ, zmq.REP) zmq_socket_type = getattr(zmq, socket_type_str.upper()) - zmq_ports[port_name] = ZeroMQPort(port_type, address, zmq_socket_type) - print(f"Initialized ZMQ port: {port_name} ({socket_type_str}) on {address}") + normalized_address = _normalize_zmq_address(port_type, address) + zmq_ports[port_name] = ZeroMQPort(port_type, normalized_address, zmq_socket_type) + print(f"Initialized ZMQ port: {port_name} ({socket_type_str}) on {normalized_address}") except AttributeError: print(f"Error: Invalid ZMQ socket type string '{socket_type_str}'.") except zmq.error.ZMQError as e: @@ -106,7 +154,7 @@ def terminate_zmq(): def safe_literal_eval(filename, defaultValue): try: with open(filename, "r") as file: - return literal_eval(file.read()) + return _parse_untrusted_value(file.read(), defaultValue) except (FileNotFoundError, SyntaxError, ValueError, Exception) as e: # Keep print for debugging, but can be made quieter # print(f"Info: Error reading {filename} or file not found, using default: {e}") @@ -146,7 +194,8 @@ def safe_literal_eval(filename, defaultValue): sparams = "{'"+re.sub(';',",'",re.sub('=',"':",re.sub(' ','',sparams)))+"}" print("converted sparams: " + sparams) try: - params = literal_eval(sparams) + parsed_params = _parse_untrusted_value(sparams, dict()) + params = parsed_params if isinstance(parsed_params, dict) else dict() except Exception as e: print(f"bad params content: {sparams}, error: {e}") params = dict() @@ -219,8 +268,12 @@ def read(port_identifier, name, initstr_val): print(f"Error: Invalid port identifier '{port_identifier}' for file operation. Must be integer or ZMQ name.") return default_return_val - time.sleep(delay) - file_path = os.path.join(inpath+str(file_port_num), name) + time.sleep(delay) + try: + file_path = _safe_channel_path(inpath, file_port_num, name) + except ValueError as e: + print(f"Error: {e}") + return default_return_val ins = "" try: @@ -253,7 +306,7 @@ def read(port_identifier, name, initstr_val): # Try parsing try: - inval = literal_eval(ins) + inval = _parse_untrusted_value(ins, default_return_val) if isinstance(inval, list) and len(inval) > 0: current_simtime_from_file = inval[0] if isinstance(current_simtime_from_file, (int, float)): @@ -320,7 +373,7 @@ def initval(simtime_val_str): """ global simtime try: - val = literal_eval(simtime_val_str) + val = _parse_untrusted_value(simtime_val_str, []) if isinstance(val, list) and len(val) > 0: first_element = val[0] if isinstance(first_element, (int, float)): diff --git a/demo/pwrap.py b/demo/pwrap.py index beb90116..2fccd59d 100644 --- a/demo/pwrap.py +++ b/demo/pwrap.py @@ -61,7 +61,10 @@ except: init_simtime_ym = "[0.0, 0.0, 0.0]" -print(apikey) +if apikey: + print('apikey loaded') +else: + print('apikey not found') print(yuyu) print(name1+'='+init_simtime_u) print(name2+'='+init_simtime_ym) diff --git a/ratc/pwrap.py b/ratc/pwrap.py index beb90116..2fccd59d 100644 --- a/ratc/pwrap.py +++ b/ratc/pwrap.py @@ -61,7 +61,10 @@ except: init_simtime_ym = "[0.0, 0.0, 0.0]" -print(apikey) +if apikey: + print('apikey loaded') +else: + print('apikey not found') print(yuyu) print(name1+'='+init_simtime_u) print(name2+'='+init_simtime_ym)