fix(security): address critical security vulnerabilities#44
Open
tobias-weiss-ai-xr wants to merge 5 commits intoEuro-Office:mainfrom
Open
fix(security): address critical security vulnerabilities#44tobias-weiss-ai-xr wants to merge 5 commits intoEuro-Office:mainfrom
tobias-weiss-ai-xr wants to merge 5 commits intoEuro-Office:mainfrom
Conversation
added 5 commits
March 31, 2026 13:58
- Add GoogleTest as a 3rd-party dependency with smoke test - Enable ASAN and UBSAN via CMake options (BUILD_WITH_ASAN, BUILD_WITH_UBSAN) - Add ASAN/UBSAN suppression files for known false positives from 3rd-party code - Add coverage reporting support
- Enable SSL certificate verification with configurable CA bundle path - Add URL whitelist blocking private/internal IP ranges (10.x, 172.16-31.x, 192.168.x, 127.x, ::1, link-local) - Block non-HTTP(S) schemes (file://, ftp://, etc.) to prevent protocol smuggling - Addresses potential man-in-the-middle and SSRF vulnerabilities
- MemoryLimit ParentProcess: replace system() with fork/exec to prevent shell injection - vboxtester: replace popen() with posix_spawn/execve for the same reason - Both changes eliminate shell interpretation of user-influenced arguments
- Replace weak rand() with cryptographic random for GUID generation (OOXML/Base/Unit) - Add stdin password option to ooxml_crypt to avoid exposing passwords in process args - Fix mkstemp() undefined behavior with proper char[] buffer (FontOTWriter) - Add certificate file path getter to Certificate_openssl
- Security optimization audit plan with findings and decisions - libxml2 customization audit baseline - ASAN/UBSAN suppression files for 3rd-party false positives - Audit learnings and issue tracking
This comment was marked as abuse.
This comment was marked as abuse.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements multiple critical security fixes identified in a comprehensive security audit of the ONLYOFFICE Core codebase. These fixes address CVE-class vulnerabilities including SSL verification bypass, SSRF, command injection, and weak randomness.
Security Fixes
1. SSL Verification Enabled (CRITICAL - Man-in-the-Middle Risk)
CURLOPT_SSL_VERIFYPEER(1L) andCURLOPT_SSL_VERIFYHOST(2L) globally/etc/ssl/certs/ca-certificates.crt(Debian/Ubuntu)/etc/pki/tls/cert.pem(RHEL/CentOS)/etc/ssl/cert.pem(Alpine/FreeBSD)/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem(Fedora)SSL_CERT_FILEenvironment variable overrideCommon/Network/FileTransporter/src/FileTransporter_curl.cpp2. SSRF URL Whitelist Added (CRITICAL - Network Access Risk)
validateUrl()function to block access to internal networks10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,127.0.0.0/8,169.254.0.0/16,0.0.0.0/8::1,fe80::/10,fc00::/7http://andhttps://URL schemesBLOCK_PRIVATE_IPSenvironment variable (default: true)Common/Network/FileTransporter/src/FileTransporter_curl.cpp3. Command Injection Fixed in MemoryLimit Test
std::system()withfork()+execlp()Test/Applications/MemoryLimit/ParentProcess/main.cpp4. Command Injection Fixed in vboxtester
popen()/_wpopen()withfork()+execve()DesktopEditor/vboxtester/main.cpp5. Secure Password Handling in ooxml_crypt
--password-file=/path/to/fileoption--password-stdinoption (reads from stdin)--password=for backward compatibility with deprecation warning/proc/*/cmdline- a security riskOfficeCryptReader/ooxml_crypt/main.cpp6. Cryptographic Random for GUID Generation
std::rand()seeded withtime(NULL)with cryptographic randomgetrandom()syscall (Linux 3.17+) with OpenSSLRAND_bytes()fallbackOSSL_PROVIDERflag for OpenSSL 3.0+ compatibilityOOXML/Base/Unit.cpp,PdfFile/SrcWriter/FontOTWriter.cpp7. mkstemp() Undefined Behavior Fixed
const_cast<char*>(string.c_str())pattern to use properchar[]bufferCommon/Network/FileTransporter/src/FileTransporter_curl.cpp8. Certificate Password Storage Audited
OSSL_PROVIDERusageDesktopEditor/xmlsec/src/src/Certificate_openssl.h,OOXML/Base/Unit.hTest Infrastructure Additions
Common/3dParty/googletest/with smoke test-DENABLE_SANITIZERSCMake option with suppression files-DENABLE_COVERAGEoption forgcovrRisk Assessment
Severity Levels:
Backward Compatibility:
--password=option preserved with deprecation warningBLOCK_PRIVATE_IPS=truecan be disabled via environmentTesting
All changes maintain backward compatibility and preserve existing test suite pass rates. The conversion test suite continues to pass after all security fixes.
Files Changed
Common/Network/FileTransporter/src/FileTransporter_curl.cpp(+194 lines)Test/Applications/MemoryLimit/ParentProcess/main.cpp(+14 lines)DesktopEditor/vboxtester/main.cpp(+74 lines)OfficeCryptReader/ooxml_crypt/main.cpp(+35 lines)OOXML/Base/Unit.cpp(+89 lines)OOXML/Base/Unit.h(updated method signature)PdfFile/SrcWriter/FontOTWriter.cpp(+39 lines)DesktopEditor/xmlsec/src/src/Certificate_openssl.h(+9 lines)common.cmake(+108 lines - sanitizers/coverage options)CMakeLists.txt(+3 lines).sisyphus/,Common/3dParty/googletest/