fix(security): address critical security vulnerabilities (SSL, SSRF, command injection)#1642
Conversation
- 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
|
|
|
While reviewing this project, I found it to be well designed overall, however I identified several security vulnerabilities. unfortunately, there is no Private Vulnerability Reporting mechanism available in the repository, which limits the ability to disclose these issues responsibly outside of public pull requests. I have discovered over 10 security issues, including critical and high-severity vulnerabilities such as potential RCE, feel free to contact me for further details if you are planning to address the issue. |
|
Hello @bohmiiidd, please send us the information about vulnerabilities found and how they affect product functioning to security@onlyoffice.com. You can also apply to our bug bounty program on HackerOne, please let us know your HackerOne nickname and we will invite you there. |
Summary
This PR addresses multiple critical security vulnerabilities identified during a comprehensive security audit of the ONLYOFFICE Core codebase. These issues include CVE-class vulnerabilities that could allow Man-in-the-Middle attacks, Server-Side Request Forgery, and command injection.
Security Fixes
1. SSL Certificate Verification Enabled (Critical — MITM Risk)
File:
Common/Network/FileTransporter/src/FileTransporter_curl.cppCURLOPT_SSL_VERIFYPEERwas set to0on Linux, completely disabling SSL certificate verification. This allowed any attacker performing a Man-in-the-Middle attack to intercept and modify HTTPS traffic (remote document references, template downloads, etc.) without detection.CURLOPT_SSL_VERIFYPEER(1L) andCURLOPT_SSL_VERIFYHOST(2L) globally with a configurable CA bundle path supporting multiple platforms:/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 override2. SSRF URL Whitelist (Critical — Network Access Risk)
File:
Common/Network/FileTransporter/src/FileTransporter_curl.cpphttp://169.254.169.254/latest/meta-data/for cloud metadata, internal APIs, etc.).validateUrl()function that:10.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)3. Command Injection Fixes (Critical)
Files:
Test/Applications/MemoryLimit/ParentProcess/main.cppDesktopEditor/vboxtester/main.cppProblem:
std::system()andpopen()/_wpopen()were used with string-concatenated commands, allowing shell metacharacter injection through user-influenced arguments.Fix: Replaced with
fork()+execve()/posix_spawn()using explicit argument arrays, eliminating shell interpretation entirely.4. Cryptographic Randomness for GUID Generation (High)
Files:
OOXML/Base/Unit.cpp,PdfFile/SrcWriter/FontOTWriter.cppstd::rand()seeded withtime(NULL), making them predictable and potentially exploitable.getrandom()syscall (Linux 3.17+) withRAND_bytes()fallback, enablingOSSL_PROVIDERfor OpenSSL 3.0+ compatibility.5. Secure Password Handling (High)
File:
OfficeCryptReader/ooxml_crypt/main.cpp--password=VALUEare visible in/proc/*/cmdlineon Linux.--password-file=/path/to/fileand--password-stdinoptions. Kept--password=for backward compatibility with a deprecation warning.6. mkstemp() Undefined Behavior Fix (Medium)
File:
Common/Network/FileTransporter/src/FileTransporter_curl.cppconst_cast<char*>(string.c_str())passed tomkstemp()causes undefined behavior (modifying string internal data).char[]buffer formkstemp().Migration Notes for Administrators
If you previously relied on disabled SSL verification (e.g., for self-signed certificates or internal IPs), you have several options:
SSL_CERT_FILEenvironment variable to point to your custom CA bundleBLOCK_PRIVATE_IPS=falseif you need to access internal IP addresses (not recommended for internet-facing deployments)Files Changed
Common/Network/FileTransporter/src/FileTransporter_curl.cppTest/Applications/MemoryLimit/ParentProcess/main.cppDesktopEditor/vboxtester/main.cppOfficeCryptReader/ooxml_crypt/main.cppOOXML/Base/Unit.cppOOXML/Base/Unit.hPdfFile/SrcWriter/FontOTWriter.cppDesktopEditor/xmlsec/src/src/Certificate_openssl.hBackward Compatibility
All changes maintain backward compatibility:
--password=option preserved with deprecation warningBLOCK_PRIVATE_IPScan be disabled via environment variableSSL_CERT_FILEoverrideRelated