diff --git a/.bandit/bandit.config b/.bandit/bandit.config new file mode 100644 index 00000000..8d0c07f3 --- /dev/null +++ b/.bandit/bandit.config @@ -0,0 +1,394 @@ +### This config may optionally select a subset of tests to run or skip by +### filling out the 'tests' and 'skips' lists given below. If no tests are +### specified for inclusion then it is assumed all tests are desired. The skips +### set will remove specific tests from the include set. This can be controlled +### using the -t/-s CLI options. Note that the same test ID should not appear +### in both 'tests' and 'skips', this would be nonsensical and is detected by +### Bandit at runtime. + +# Available tests: +# B101 : assert_used +# B102 : exec_used +# B103 : set_bad_file_permissions +# B104 : hardcoded_bind_all_interfaces +# B105 : hardcoded_password_string +# B106 : hardcoded_password_funcarg +# B107 : hardcoded_password_default +# B108 : hardcoded_tmp_directory +# B110 : try_except_pass +# B112 : try_except_continue +# B201 : flask_debug_true +# B301 : pickle +# B302 : marshal +# B303 : md5 +# B304 : ciphers +# B305 : cipher_modes +# B306 : mktemp_q +# B307 : eval +# B308 : mark_safe +# B310 : urllib_urlopen +# B311 : random +# B312 : telnetlib +# B313 : xml_bad_cElementTree +# B314 : xml_bad_ElementTree +# B315 : xml_bad_expatreader +# B316 : xml_bad_expatbuilder +# B317 : xml_bad_sax +# B318 : xml_bad_minidom +# B319 : xml_bad_pulldom +# B321 : ftplib +# B323 : unverified_context +# B324 : hashlib_new_insecure_functions +# B401 : import_telnetlib +# B402 : import_ftplib +# B403 : import_pickle +# B404 : import_subprocess +# B405 : import_xml_etree +# B406 : import_xml_sax +# B407 : import_xml_expat +# B408 : import_xml_minidom +# B409 : import_xml_pulldom +# B411 : import_xmlrpclib +# B412 : import_httpoxy +# B413 : import_pycrypto +# B501 : request_with_no_cert_validation +# B502 : ssl_with_bad_version +# B503 : ssl_with_bad_defaults +# B504 : ssl_with_no_version +# B505 : weak_cryptographic_key +# B506 : yaml_load +# B507 : ssh_no_host_key_verification +# B601 : paramiko_calls +# B602 : subprocess_popen_with_shell_equals_true +# B603 : subprocess_without_shell_equals_true +# B604 : any_other_function_with_shell_equals_true +# B605 : start_process_with_a_shell +# B606 : start_process_with_no_shell +# B607 : start_process_with_partial_path +# B608 : hardcoded_sql_expressions +# B609 : linux_commands_wildcard_injection +# B610 : django_extra_used +# B611 : django_rawsql_used +# B701 : jinja2_autoescape_false +# B702 : use_of_mako_templates +# B703 : django_mark_safe + +# (optional) list included test IDs here, eg '[B101, B406]': +# Required security checkers - do not disable these +# Additional checkers may be added if desired +tests: + [ 'B301', 'B302', 'B303', 'B304', 'B305', 'B306', 'B308', 'B310', 'B311', 'B312', 'B313', 'B314', 'B315', 'B316', 'B317', 'B318', 'B319', 'B321', 'B323', 'B324', 'B401', 'B402', 'B403', 'B404', 'B405', 'B406', 'B407', 'B408', 'B409', 'B411', 'B412', 'B413'] + +# (optional) list skipped test IDs here, eg '[B101, B406]': +# Optional checkers - may be added to tests list if desired +skips: + [ 'B101', 'B102', 'B103', 'B104', 'B105', 'B106', 'B107', 'B108', 'B110', 'B112', 'B201', 'B501', 'B502', 'B503', 'B504', 'B505', 'B506', 'B507', 'B601', 'B602', 'B603', 'B604', 'B605', 'B606', 'B607', 'B608', 'B609', 'B610', 'B611', 'B701', 'B702', 'B703'] + +### (optional) plugin settings - some test plugins require configuration data +### that may be given here, per-plugin. All bandit test plugins have a built in +### set of sensible defaults and these will be used if no configuration is +### provided. It is not necessary to provide settings for every (or any) plugin +### if the defaults are acceptable. + +any_other_function_with_shell_equals_true: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +assert_used: + skips: [] +hardcoded_tmp_directory: + tmp_dirs: + - /tmp + - /var/tmp + - /dev/shm +linux_commands_wildcard_injection: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +ssl_with_bad_defaults: + bad_protocol_versions: + - PROTOCOL_SSLv2 + - SSLv2_METHOD + - SSLv23_METHOD + - PROTOCOL_SSLv3 + - PROTOCOL_TLSv1 + - SSLv3_METHOD + - TLSv1_METHOD +ssl_with_bad_version: + bad_protocol_versions: + - PROTOCOL_SSLv2 + - SSLv2_METHOD + - SSLv23_METHOD + - PROTOCOL_SSLv3 + - PROTOCOL_TLSv1 + - SSLv3_METHOD + - TLSv1_METHOD +start_process_with_a_shell: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +start_process_with_no_shell: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +start_process_with_partial_path: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +subprocess_popen_with_shell_equals_true: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +subprocess_without_shell_equals_true: + no_shell: + - os.execl + - os.execle + - os.execlp + - os.execlpe + - os.execv + - os.execve + - os.execvp + - os.execvpe + - os.spawnl + - os.spawnle + - os.spawnlp + - os.spawnlpe + - os.spawnv + - os.spawnve + - os.spawnvp + - os.spawnvpe + - os.startfile + shell: + - os.system + - os.popen + - os.popen2 + - os.popen3 + - os.popen4 + - popen2.popen2 + - popen2.popen3 + - popen2.popen4 + - popen2.Popen3 + - popen2.Popen4 + - commands.getoutput + - commands.getstatusoutput + subprocess: + - subprocess.Popen + - subprocess.call + - subprocess.check_call + - subprocess.check_output + - subprocess.run +try_except_continue: + check_typed_exception: false +try_except_pass: + check_typed_exception: false +weak_cryptographic_key: + weak_key_size_dsa_high: 1024 + weak_key_size_dsa_medium: 2048 + weak_key_size_ec_high: 160 + weak_key_size_ec_medium: 224 + weak_key_size_rsa_high: 1024 + weak_key_size_rsa_medium: 2048 diff --git a/.github/workflows/ci-bandit.yml b/.github/workflows/ci-bandit.yml new file mode 100644 index 00000000..07a9fd38 --- /dev/null +++ b/.github/workflows/ci-bandit.yml @@ -0,0 +1,36 @@ +name: bandit-security-scan + +on: + push: + branches: [ '**' ] + paths: + - '**.py' + - '.bandit/**' + - '.github/workflows/ci-bandit.yml' + pull_request: + branches: [ '**' ] + paths: + - '**.py' + - '.bandit/**' + - '.github/workflows/ci-bandit.yml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + bandit: + runs-on: ci-test + if: ${{ github.repository != 'intel/pcm' }} + + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0 + with: + python-version: '3.12' + + - name: Install and run Bandit + run: | + pip install bandit + bandit -r . -c .bandit/bandit.config -x ./Intel-PMT,./perfmon,./src/pugixml,./src/simdjson,./tests/googletest -f txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 1bdc5063..73f81465 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -67,9 +67,9 @@ if(UNIX) # APPLE, LINUX, FREE_BSD set (PCM_COMMON_FLAGS "-Wno-unknown-pragmas -DCMAKE_INSTALL_PREFIX=\"${CMAKE_INSTALL_PREFIX}\"") if(LINUX) - set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wextra -DPCM_USE_PERF") - if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") - set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wl,-z,now") + set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wextra -DPCM_USE_PERF -fstack-protector-strong") + if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") + set (PCM_COMMON_FLAGS "${PCM_COMMON_FLAGS} -Wl,-z,now -Wl,-z,relro -Wl,-z,noexecstack") endif() if(NOT DEFINED LINUX_SYSTEMD) diff --git a/scripts/pcm-win-power-tray.py b/scripts/pcm-win-power-tray.py index 84fa7ed1..f0b59807 100644 --- a/scripts/pcm-win-power-tray.py +++ b/scripts/pcm-win-power-tray.py @@ -2,7 +2,7 @@ from PIL import Image, ImageDraw, ImageFont import threading import time -import subprocess +import subprocess # nosec B404 - subprocess used for controlled system commands, no user input import csv import io diff --git a/scripts/pmu-query.py b/scripts/pmu-query.py index 9a980081..8f9fcf5c 100644 --- a/scripts/pmu-query.py +++ b/scripts/pmu-query.py @@ -46,17 +46,17 @@ break if platform.system() == "CYGWIN_NT-6.1": - p = subprocess.Popen(["./pcm-core.exe -c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen(["./pcm-core.exe", "-c"], stdout=subprocess.PIPE, shell=False) elif platform.system() == "Windows": - p = subprocess.Popen(["pcm-core.exe", "-c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen(["pcm-core.exe", "-c"], stdout=subprocess.PIPE, shell=False) elif platform.system() == "Linux": pcm_core = shutil.which("pcm-core") if not pcm_core: print("Could not find pcm-core executable!") sys.exit(-1) - p = subprocess.Popen([pcm_core, "-c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen([pcm_core, "-c"], stdout=subprocess.PIPE, shell=False) else: - p = subprocess.Popen(["../build/bin/pcm-core -c"], stdout=subprocess.PIPE, shell=True) + p = subprocess.Popen(["../build/bin/pcm-core", "-c"], stdout=subprocess.PIPE, shell=False) (output, err) = p.communicate() p_status = p.wait() diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 79458f01..c19d30db 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -99,8 +99,14 @@ if(UNIX) # LINUX, FREE_BSD, APPLE endif() if(MSVC) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /O2 /wd4251 /wd4273 /EHa /Zi") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /O2 /wd4251 /wd4273 /EHa /Zi /GS /Gy") add_definitions(/W3) + # SDL429 security flags: ASLR (/DYNAMICBASE) and DEP (/NXCOMPAT) + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /DYNAMICBASE /NXCOMPAT") + # Optional Spectre mitigation for newer MSVC versions + if(MSVC_TOOLSET_VERSION GREATER_EQUAL 141) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Qspectre") + endif() # windows/* files -> PCM_STATIC file(GLOB WINDOWS_SOURCES winpmem/winpmem.cpp windows/stdafx.cpp freegetopt/getopt.cpp) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 0a0bee20..d64318d7 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -313,17 +314,41 @@ namespace PCMDaemon { exit(EXIT_FAILURE); } - //Store shm id in a file (shmIdLocation_) - int success = remove(shmIdLocation_.c_str()); - if (success != 0) - { - std::cerr << "Failed to delete shared memory id location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; + // Store shm id in a file (shmIdLocation_) + // SDL330: Atomic file creation with symlink protection + // Try O_EXCL first, unlink and retry only if needed (avoids TOCTOU race) + int fd = -1; + constexpr int MAX_FILE_CREATION_RETRIES = 3; + for (int attempt = 0; attempt < MAX_FILE_CREATION_RETRIES && fd < 0; ++attempt) { + fd = open(shmIdLocation_.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_NOFOLLOW, 0660); + if (fd >= 0) break; + + if (errno == ELOOP) { + std::cerr << "SDL330 CRITICAL: Symlink detected at " << shmIdLocation_ << "\n"; + exit(EXIT_FAILURE); + } + if (errno == EEXIST) { + // File exists from previous run - unlink and retry + if (unlink(shmIdLocation_.c_str()) != 0 && errno != ENOENT) { + std::cerr << "Failed to delete stale shared memory id file: " << shmIdLocation_ << "\n"; + exit(EXIT_FAILURE); + } + continue; // retry + } + // Other error + std::cerr << "Failed to create shared memory key location: " << shmIdLocation_ << " (errno=" << errno << ")\n"; + exit(EXIT_FAILURE); + } + if (fd < 0) { + std::cerr << "SDL330 CRITICAL: Unable to create shared memory file after retries (possible attack): " << shmIdLocation_ << "\n"; + exit(EXIT_FAILURE); } - FILE* fp = fopen(shmIdLocation_.c_str(), "w"); + FILE* fp = fdopen(fd, "w"); if (!fp) { - std::cerr << "Failed to create/write to shared memory key location: " << shmIdLocation_ << "\n"; + close(fd); + std::cerr << "Failed to open stream for shared memory key location: " << shmIdLocation_ << "\n"; exit(EXIT_FAILURE); } fprintf(fp, "%i", sharedMemoryId_); @@ -337,7 +362,7 @@ namespace PCMDaemon { shmData.shm_perm.gid = gid; shmData.shm_perm.mode = mode; - success = shmctl(sharedMemoryId_, IPC_SET, &shmData); + int success = shmctl(sharedMemoryId_, IPC_SET, &shmData); if (success < 0) { std::cerr << "Failed to IPC_SET (errno=" << errno << ")\n"; @@ -749,7 +774,7 @@ namespace PCMDaemon { else { // Delete segment - success = shmctl(sharedMemoryId_, IPC_RMID, NULL); + int success = shmctl(sharedMemoryId_, IPC_RMID, NULL); if (success != 0) { std::cerr << "Failed to delete the shared memory segment (errno=" << errno << ")\n"; diff --git a/src/msr.cpp b/src/msr.cpp index cdbfc433..2709c5b0 100644 --- a/src/msr.cpp +++ b/src/msr.cpp @@ -166,8 +166,13 @@ MsrHandle::MsrHandle(uint32 cpu) : fd(-1), cpu_id(cpu) { char path[200]; snprintf(path, 200, "/dev/cpuctl%u", cpu); - int handle = ::open(path, O_RDWR); - if (handle < 0) throw std::exception(); + int handle = ::open(path, O_RDWR | O_NOFOLLOW); + if (handle < 0) { + if (errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at " << path << "\n"; + } + throw std::exception(); + } fd = handle; } @@ -229,11 +234,17 @@ MsrHandle::MsrHandle(uint32 cpu) : fd(-1), cpu_id(cpu) char * path = new char[200]; if (!path) throw std::runtime_error("Allocation of 200 bytes failed."); snprintf(path, 200, "/dev/cpu/%u/msr", cpu); - int handle = ::open(path, O_RDWR); + int handle = ::open(path, O_RDWR | O_NOFOLLOW); + if (handle < 0 && errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at MSR device path " << path << "\n"; + } if (handle < 0) { // try Android msr device path snprintf(path, 200, "/dev/msr%u", cpu); - handle = ::open(path, O_RDWR); + handle = ::open(path, O_RDWR | O_NOFOLLOW); + if (handle < 0 && errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at MSR device path " << path << "\n"; + } } deleteAndNullifyArray(path); if (handle < 0) diff --git a/src/pci.cpp b/src/pci.cpp index 89a852ab..86c02efc 100644 --- a/src/pci.cpp +++ b/src/pci.cpp @@ -258,8 +258,13 @@ PciHandle::PciHandle(uint32 groupnr_, uint32 bus_, uint32 device_, uint32 functi device(device_), function(function_) { - int handle = ::open("/dev/pci", O_RDWR); - if (handle < 0) throw std::exception(); + int handle = ::open("/dev/pci", O_RDWR | O_NOFOLLOW); + if (handle < 0) { + if (errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at /dev/pci\n"; + } + throw std::exception(); + } fd = handle; } @@ -271,8 +276,13 @@ bool PciHandle::exists(uint32 groupnr_, uint32 bus_, uint32 device_, uint32 func int fd; int ret; - fd = ::open("/dev/pci", O_RDWR, 0); - if (fd < 0) return false; + fd = ::open("/dev/pci", O_RDWR | O_NOFOLLOW, 0); + if (fd < 0) { + if (errno == ELOOP) { + std::cerr << "SDL330 ERROR: Symlink detected at /dev/pci\n"; + } + return false; + } bzero(&pc, sizeof(pc)); @@ -391,11 +401,15 @@ int openHandle(uint32 groupnr_, uint32 bus, uint32 device, uint32 function) // std::cout << "PciHandle: Opening "<isActiveRelativeFrequencyAvailable()) { - cout << " CFREQ : core frequency in Ghz\n"; + cout << " CFREQ : core frequency in GHz\n"; } break; default: @@ -1651,4 +1651,4 @@ int mainThrows(int argc, char * argv[]) exit(EXIT_SUCCESS); } -#endif // UNIT_TEST \ No newline at end of file +#endif // UNIT_TEST diff --git a/src/tpmi.cpp b/src/tpmi.cpp index 07aacfda..81c6b129 100644 --- a/src/tpmi.cpp +++ b/src/tpmi.cpp @@ -57,7 +57,7 @@ class PFSInstances assert(vsec.fields.EntrySize == 2); std::vector pfsArray(vsec.fields.NumEntries); try { - mmio_memcpy(&(pfsArray[0]), bar + vsec.fields.Address, vsec.fields.NumEntries * sizeof(PFS), true, true); + mmio_memcpy(&(pfsArray[0]), bar + 8ULL * vsec.fields.Address, vsec.fields.NumEntries * sizeof(PFS), true, true); } catch (std::runtime_error & e) { std::cerr << "Can't read PFS\n"; @@ -79,7 +79,7 @@ class PFSInstances for (uint64 p = 0; p < pfs.NumEntries; ++p) { uint32 reg0 = 0; - const auto addr = bar + vsec.fields.Address + pfs.CapOffset * 1024ULL + p * pfs.EntrySize * sizeof(uint32); + const auto addr = bar + 8ULL * vsec.fields.Address + pfs.CapOffset * 1024ULL + p * pfs.EntrySize * sizeof(uint32); try { mmio_memcpy(®0, addr, sizeof(uint32), false, true); } catch (std::runtime_error & e) diff --git a/src/utils.cpp b/src/utils.cpp index f1a3a305..64aac699 100644 --- a/src/utils.cpp +++ b/src/utils.cpp @@ -17,6 +17,7 @@ #else #include // for waitpid() #include // for ::sleep +#include // for open(), O_NOFOLLOW #endif #include "utils.h" #include "cpucounters.h" @@ -1500,10 +1501,28 @@ std::pair parseBitsParameter(const char * param) #ifdef __linux__ FILE * tryOpen(const char * path, const char * mode) { - FILE * f = fopen(path, mode); - if (!f) + // SDL330: O_NOFOLLOW rejects symlinks atomically (no TOCTOU) + int flags = O_NOFOLLOW; + if (strchr(mode, 'w')) { + flags |= O_WRONLY | O_CREAT | O_TRUNC; + } else { + flags |= O_RDONLY; + } + + int fd = open(path, flags, 0644); + if (fd < 0) { - f = fopen((std::string("/pcm") + path).c_str(), mode); + const std::string alt_path = std::string("/pcm") + path; + fd = open(alt_path.c_str(), flags, 0644); + } + + if (fd < 0) { + return nullptr; + } + + FILE * f = fdopen(fd, mode); + if (!f) { + close(fd); } return f; } diff --git a/src/winring0/OlsApiInit.h b/src/winring0/OlsApiInit.h index fd9383b0..da67ab04 100644 --- a/src/winring0/OlsApiInit.h +++ b/src/winring0/OlsApiInit.h @@ -135,13 +135,22 @@ _SetOlsValue SetOlsValue = NULL; BOOL InitOpenLibSys(HMODULE *hModule) { TCHAR dll_path[MAX_PATH]; - GetSystemDirectory(dll_path, MAX_PATH - 20); + // SDL436: Use fully qualified path from System32 directory + // GetSystemDirectory ensures we load from the trusted System32 location + if (!GetSystemDirectory(dll_path, MAX_PATH - 20)) + { + std::wcerr << "Failed to get System32 directory path.\n"; + return FALSE; + } #ifdef _M_X64 _tcscat_s(dll_path, MAX_PATH, TEXT("\\WinRing0x64.dll")); #else _tcscat_s(dll_path, MAX_PATH, TEXT("\\WinRing0.dll")); #endif - *hModule = LoadLibrary(dll_path); + + // SDL436: Use LoadLibraryEx with LOAD_LIBRARY_SEARCH_SYSTEM32 flag + // This ensures secure DLL loading from System32 only, preventing DLL injection + *hModule = LoadLibraryEx(dll_path, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); if(*hModule == NULL) {