fix(security): harden URL handling, fix memory safety and race conditions#721
Open
grisutheguru wants to merge 3 commits intoTokTok:masterfrom
Open
fix(security): harden URL handling, fix memory safety and race conditions#721grisutheguru wants to merge 3 commits intoTokTok:masterfrom
grisutheguru wants to merge 3 commits intoTokTok:masterfrom
Conversation
…ions - Block dangerous URI schemes (smb://, file://, ed2k://) in chat links and show confirmation dialog before opening any external URL to protect Tor/proxy users from IP deanonymization - Block remote resource loading in CustomTextDocument (only allow key:/qrc:) - Fix use-after-free: replace delete-this with deleteLater() in CoreVideoSource, fix CameraDevice::close() lock ordering - Fix TOCTOU race in VideoFrame::fromAVFrame lock upgrade - Initialize ReadWriteLocker::lock_ to nullptr (was uninitialized) - Fix cancelCall() race: check isCancelling under callsLock in callbacks - Wipe encryption key material in PassKeyDeleter and deriveKey() - Use async-signal-safe functions only in crash/signal handlers - Replace rand()/clock-seeded mt19937 with std::random_device/QRandomGenerator - Fix conference title logic bug (|| vs &&) - Fix IPC busy-wait (msleep(50) + 30s cap), use getpwuid() over getenv - Atomic rename() on POSIX for database file swap - Add Profile::isValidProfileName() to prevent path traversal - Add Paths::setSecureFilePermissions() for sensitive files (0600) - Fix ReDoS in textformatter regex, remove dangerous URI patterns - Validate VPX frame dimensions, add uint16_t bounds check - Prevent OpenAL inputBuffer double-free Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Tip Preview URL: |
github-actions
Bot
added
the
bug
Tox_Pass_Key is an opaque/incomplete type in toxcore's public API, so sizeof() cannot be applied. Remove the manual wipe in PassKeyDeleter (tox_pass_key_free handles cleanup). The intermediate buffer wipes in deriveKey() are preserved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
QMutex is non-copyable, so operator[]= {} fails. Use bare operator[]
which default-constructs the entry in-place.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Security hardening across 16 files addressing vulnerabilities found during a comprehensive code audit.
URL / Link Handling (remote-exploitable)
smb://,file://,ed2k://,ms-msdt:,search-ms:) in chat message links — prevents NTLM hash theft on Windows and local file probingQDesktopServices::openUrl()opens the system browser which bypasses the Tox SOCKS5 proxy, revealing the user's real IP addressCustomTextDocument::loadResource()— only allowkey:(smileys) andqrc:schemes, preventing IP tracking via injected<img>tagsfile://,smb://,ed2k://from clickable link regex intextformatter.cpp[\S| ]character class, add length capMemory Safety
CoreVideoSource::unsubscribe(): replacedelete thiswithdeleteLater()CameraDevice::close(): acquireopenDeviceLockbefore decrementingrefcountReadWriteLocker::lock_: add= nullptrinputBuffer = nullptrafterdelete[]inOpenAL::cleanupInput()uint16_tcast of video frame dimensionsRace Conditions
VideoFrame::fromAVFrame: re-check after read-to-write lock upgradecancelCall()race: set/checkisCancellingundercallsLockCrypto / Privacy
PassKeyDeleterzeroes key;deriveKey()wipes intermediate buffersPaths::setSecureFilePermissions()(0600) applied to.toxsave filesPlatform / Hardening
write(STDERR_FILENO)+_exit()instead ofqCritical()/exit()std::random_deviceseed;QRandomGeneratorinstead ofrand()msleep(50)+ 30s cap instead ofmsleep(0)infinite loopgetpwuid(getuid())on POSIX instead ofgetenv("USER")rename()on POSIX with error checkingisValidProfileName()rejects path traversal and reserved names||to&&inloadConferences()Test plan
tox:URI in chat — should open directly without dialoghttps://link in chat — should show confirmation dialog with IP warningsmb://orfile://link — should be silently blocked../,CON,NUL— should be rejected🤖 Generated with Claude Code
This change is