#3 Fixed Issue #443 + Detection Prevention + WebRTC Re-enablement #8
Conversation
…urrency, and timezone (daijro#443) Fixes issue where global CAMOU_CONFIG settings for navigator.platform, navigator.hardwareConcurrency, and timezone were not applied in the main window Navigator (only WorkerNavigator was patched via fingerprint-injection.patch).
…ialized to prevent SIGSEGV
- navigator-spoofing.patch: Global config fixes for platform/hardwareConcurrency/timezone - timezone-spoofing.patch: Persistence across page navigation via SetNewDocument hook - audio-fingerprint-manager.patch: Full 6-method coverage with self-destruct pattern - screen-spoofing.patch: Per-context dimensions with self-destruct pattern - webrtc-ip-spoofing.patch: Re-enabled with getStats() sanitization + comprehensive IPv6 regex
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Paragon SummaryThis pull request review identified 10 issues across 2 categories in 5 files. The review analyzed code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools. Fixes Issue daijro#443 by enabling global navigator config spoofing, re-enables WebRTC IP spoofing with leak prevention, and adds self-destruct patterns across fingerprint patches to prevent detection. Key changes:
Confidence score: 3/5
5 files reviewed, 10 comments Severity breakdown: High: 2, Medium: 8 Tip: |
![paragon-review[bot]](https://avatars.githubusercontent.com/in/1544258?s=60&v=4){kind=small}
| + | ||
| + mLocalRequestedSDP = sanitizedSDP; | ||
|
|
There was a problem hiding this comment.
Bug: Race condition in getStats callback closure capturing mWindow
Race condition in getStats callback closure capturing mWindow. Window pointer may become dangling or null before lambda executes on main thread.
View Details
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Analysis
Race condition in getStats callback closure capturing mWindow
| What fails | Raw window pointer captured in lambda without lifetime safety |
| Result | Captured window pointer may be invalid |
| Expected | Use weak reference or proper lifetime management for window in async callback |
| Impact | Potential use-after-free or null dereference when window is closed before callback |
How to reproduce
Close window while GetStats is pending, then wait for callbackAI Fix Prompt
Fix this issue: Race condition in getStats callback closure capturing mWindow. Window pointer may become dangling or null before lambda executes on main thread.
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Problem: Raw window pointer captured in lambda without lifetime safety
Current behavior: Captured window pointer may be invalid
Expected: Use weak reference or proper lifetime management for window in async callback
Steps to reproduce: Close window while GetStats is pending, then wait for callback
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| + | ||
| + mLocalRequestedSDP = sanitizedSDP; | ||
|
|
There was a problem hiding this comment.
Bug: IPv6 detection in getStats uses FindChar(':') which may incorrectly match IPv4
IPv6 detection in getStats uses FindChar(':') which may incorrectly match IPv4 addresses containing colons like '192.168.1.1:8080'. Combined with port numbers this causes incorrect address type detection.
View Details
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Analysis
**IPv6 detection in getStats uses FindChar(':') which may incorrectly match IPv4 addresses containing **
| What fails | Overly simple IPv6 detection using single colon check |
| Result | Address incorrectly detected as IPv6 due to port colon |
| Expected | Proper detection using IPv6 specific checks like multiple colons or bracketed notation |
| Impact | IPv4-like addresses with ports may be treated as IPv6, breaking sanitization |
How to reproduce
Provide stats with address '192.168.1.1:8080' and observe IPv6 handlingAI Fix Prompt
Fix this issue: IPv6 detection in getStats uses FindChar(':') which may incorrectly match IPv4 addresses containing colons like '192.168.1.1:8080'. Combined with port numbers this causes incorrect address type detection.
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Problem: Overly simple IPv6 detection using single colon check
Current behavior: Address incorrectly detected as IPv6 due to port colon
Expected: Proper detection using IPv6 specific checks like multiple colons or bracketed notation
Steps to reproduce: Provide stats with address '192.168.1.1:8080' and observe IPv6 handling
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| "BarProps.h", | ||
| @@ -326,4 +326,9 @@ SOURCES += [ | ||
| "nsWindowSizes.cpp", | ||
| ] |
There was a problem hiding this comment.
Bug: AudioBuffer::CopyFromChannel applies fingerprint transformation to destination array after copy
AudioBuffer::CopyFromChannel applies fingerprint transformation to destination array after copy. This modifies caller's buffer which may not be expected and could cause data corruption.
View Details
Location: patches/audio-fingerprint-manager.patch (lines 184)
Analysis
AudioBuffer::CopyFromChannel applies fingerprint transformation to destination array after copy
| What fails | Fingerprint transformation applied to caller's destination buffer |
| Result | Destination array contains fingerprinted data instead of original |
| Expected | Apply transformation to source data before copy, not to caller's destination buffer |
| Impact | Unexpected modification of caller's buffer could cause audio processing errors |
How to reproduce
Call copyFromChannel with destination array, then check if array was modifiedAI Fix Prompt
Fix this issue: AudioBuffer::CopyFromChannel applies fingerprint transformation to destination array after copy. This modifies caller's buffer which may not be expected and could cause data corruption.
Location: patches/audio-fingerprint-manager.patch (lines 184)
Problem: Fingerprint transformation applied to caller's destination buffer
Current behavior: Destination array contains fingerprinted data instead of original
Expected: Apply transformation to source data before copy, not to caller's destination buffer
Steps to reproduce: Call copyFromChannel with destination array, then check if array was modified
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| + | ||
| + mLocalRequestedSDP = sanitizedSDP; | ||
|
|
There was a problem hiding this comment.
Performance: Extremely complex IPv6 regex pattern may cause performance issues or miss edge cases
Extremely complex IPv6 regex pattern may cause performance issues or miss edge cases. The pattern is 400+ characters and attempts to match all IPv6 compressed formats but may have regex engine compatibility issues or exponential backtracking problems.
View Details
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Analysis
Extremely complex IPv6 regex pattern may cause performance issues or miss edge cases
| What fails | The IPv6 regex uses complex alternation with nested groups that may cause catastrophic backtracking in some regex engines or miss valid IPv6 formats. |
| Result | Potential for regex backtracking performance issues or incorrect matching on edge case IPv6 addresses. |
| Expected | Use a simpler approach or validate IPv6 addresses programmatically instead of with a single massive regex. |
| Impact | WebRTC connection performance could degrade or IPv6 addresses might leak if regex fails to match valid patterns. |
How to reproduce
Test the regex against various IPv6 formats including ::1, fe80::1, 2001:db8::1, ::ffff:192.168.1.1, and malformed inputs.AI Fix Prompt
Fix this issue: Extremely complex IPv6 regex pattern may cause performance issues or miss edge cases. The pattern is 400+ characters and attempts to match all IPv6 compressed formats but may have regex engine compatibility issues or exponential backtracking problems.
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Problem: The IPv6 regex uses complex alternation with nested groups that may cause catastrophic backtracking in some regex engines or miss valid IPv6 formats.
Current behavior: Potential for regex backtracking performance issues or incorrect matching on edge case IPv6 addresses.
Expected: Use a simpler approach or validate IPv6 addresses programmatically instead of with a single massive regex.
Steps to reproduce: Test the regex against various IPv6 formats including ::1, fe80::1, 2001:db8::1, ::ffff:192.168.1.1, and malformed inputs.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| + | ||
| + uint32_t userContextId = 0; | ||
| + | ||
| + // 1) document principal |
There was a problem hiding this comment.
Maintainability: Severe code duplication of userContextId resolution logic across multiple files
Severe code duplication of userContextId resolution logic across multiple files. The 3-level fallback pattern is copy-pasted in AudioFingerprintManager, ScreenDimensionManager, TimezoneManager, WebRTCIPManager, and nsGlobalWindowInner. This creates maintenance burden and inconsistency risk.
View Details
Location: patches/audio-fingerprint-manager.patch (lines 97)
Analysis
Severe code duplication of userContextId resolution logic across multiple files
| What fails | The same 3-level userContextId resolution pattern (document principal → docshell → top browsing context) is duplicated across 5+ files. |
| Result | Code duplication leads to maintenance overhead and potential inconsistencies if fixes are applied to only some locations. |
| Expected | Extract the userContextId resolution into a shared utility function in a common header. |
| Impact | Increases maintenance burden and risk of bugs when modifications are needed. Creates 70+ lines of duplicated code per file. |
How to reproduce
Compare code in AudioFingerprintManager.cpp:91-123, ScreenDimensionManager.cpp:91-123, TimezoneManager.cpp:29-61, WebRTCIPManager.cpp:53-61, and nsGlobalWindowInner.cpp:7431-7460.AI Fix Prompt
Fix this issue: Severe code duplication of userContextId resolution logic across multiple files. The 3-level fallback pattern is copy-pasted in AudioFingerprintManager, ScreenDimensionManager, TimezoneManager, WebRTCIPManager, and nsGlobalWindowInner. This creates maintenance burden and inconsistency risk.
Location: patches/audio-fingerprint-manager.patch (lines 97)
Problem: The same 3-level userContextId resolution pattern (document principal → docshell → top browsing context) is duplicated across 5+ files.
Current behavior: Code duplication leads to maintenance overhead and potential inconsistencies if fixes are applied to only some locations.
Expected: Extract the userContextId resolution into a shared utility function in a common header.
Steps to reproduce: Compare code in AudioFingerprintManager.cpp:91-123, ScreenDimensionManager.cpp:91-123, TimezoneManager.cpp:29-61, WebRTCIPManager.cpp:53-61, and nsGlobalWindowInner.cpp:7431-7460.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| return; | ||
| } | ||
|
|
||
| + // Apply audio fingerprint transformation to frequency data |
There was a problem hiding this comment.
Bug: AnalyserNode applies fingerprint transformation to mOutputBuffer before copying to destination
AnalyserNode applies fingerprint transformation to mOutputBuffer before copying to destination. This modifies internal buffer that may be reused, causing inconsistent results across multiple get*Data calls.
View Details
Location: patches/audio-fingerprint-manager.patch (lines 366)
Analysis
AnalyserNode applies fingerprint transformation to mOutputBuffer before copying to destination
| What fails | Fingerprint transformation applied to reusable internal buffer |
| Result | Second call returns already-fingerprinted data from mOutputBuffer |
| Expected | Apply transformation to destination array after copy, not to reusable internal buffer |
| Impact | Internal buffer modification causes inconsistent fingerprinting across calls |
How to reproduce
Call GetFloatFrequencyData twice with same seed, compare resultsAI Fix Prompt
Fix this issue: AnalyserNode applies fingerprint transformation to mOutputBuffer before copying to destination. This modifies internal buffer that may be reused, causing inconsistent results across multiple get*Data calls.
Location: patches/audio-fingerprint-manager.patch (lines 366)
Problem: Fingerprint transformation applied to reusable internal buffer
Current behavior: Second call returns already-fingerprinted data from mOutputBuffer
Expected: Apply transformation to destination array after copy, not to reusable internal buffer
Steps to reproduce: Call GetFloatFrequencyData twice with same seed, compare results
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| + | ||
| + AudioFingerprintManager::SetSeed(userContextId, seed); | ||
| + | ||
| + // Self-destruct: remove this function from the window object after first use |
There was a problem hiding this comment.
Bug: Missing null check for JSContext in SetAudioFingerprintSeed self-destruct
Missing null check for JSContext in SetAudioFingerprintSeed self-destruct. If nsContentUtils::GetCurrentJSContext() returns null, the code silently skips self-destruct but doesn't indicate failure to caller.
View Details
Location: patches/audio-fingerprint-manager.patch (lines 249)
Analysis
Missing null check for JSContext in SetAudioFingerprintSeed self-destruct
| What fails | Missing null check for JSContext in self-destruct code |
| Result | Function not removed from window but no error reported |
| Expected | Should handle null JSContext gracefully or report error |
| Impact | Inconsistent state where function remains visible after use |
How to reproduce
Call setAudioFingerprintSeed when JSContext is not availableAI Fix Prompt
Fix this issue: Missing null check for JSContext in SetAudioFingerprintSeed self-destruct. If nsContentUtils::GetCurrentJSContext() returns null, the code silently skips self-destruct but doesn't indicate failure to caller.
Location: patches/audio-fingerprint-manager.patch (lines 249)
Problem: Missing null check for JSContext in self-destruct code
Current behavior: Function not removed from window but no error reported
Expected: Should handle null JSContext gracefully or report error
Steps to reproduce: Call setAudioFingerprintSeed when JSContext is not available
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| --- a/intl/components/src/TimeZone.cpp | ||
| +++ b/intl/components/src/TimeZone.cpp | ||
| @@ -8,7 +8,8 @@ | ||
| diff --git a/dom/base/TimezoneManager.cpp b/dom/base/TimezoneManager.cpp |
There was a problem hiding this comment.
Bug: TimezoneManager::SetTimezone doesn't validate timezone string before storing
TimezoneManager::SetTimezone doesn't validate timezone string before storing. Invalid timezone identifiers are stored in RoverfoxStorageManager and only fail later when applied to JS realm.
View Details
Location: patches/timezone-spoofing.patch (lines 1)
Analysis
TimezoneManager::SetTimezone doesn't validate timezone string before storing
| What fails | Missing timezone validation in TimezoneManager::SetTimezone |
| Result | Invalid timezone stored and only fails when JS::SetRealmTimeZoneOverride called |
| Expected | Should validate timezone before storing to storage |
| Impact | Wasted storage operations and delayed error detection |
How to reproduce
Call setTimezone with invalid string like 'Invalid/Timezone'AI Fix Prompt
Fix this issue: TimezoneManager::SetTimezone doesn't validate timezone string before storing. Invalid timezone identifiers are stored in RoverfoxStorageManager and only fail later when applied to JS realm.
Location: patches/timezone-spoofing.patch (lines 1)
Problem: Missing timezone validation in TimezoneManager::SetTimezone
Current behavior: Invalid timezone stored and only fails when JS::SetRealmTimeZoneOverride called
Expected: Should validate timezone before storing to storage
Steps to reproduce: Call setTimezone with invalid string like 'Invalid/Timezone'
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| + | ||
| + mLocalRequestedSDP = sanitizedSDP; | ||
|
|
There was a problem hiding this comment.
Bug: WebRTCIPManager includes RoverfoxStorageManager
WebRTCIPManager includes RoverfoxStorageManager.h in cpp but not header. This creates inconsistent compilation units if header is included elsewhere without the required storage header.
View Details
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Analysis
WebRTCIPManager includes RoverfoxStorageManager.h in cpp but not header
| What fails | Header dependency violation - implementation header used only in .cpp |
| Result | Missing RoverfoxStorageManager.h causes undefined types |
| Expected | RoverfoxStorageManager.h should be in WebRTCIPManager.h if used in inline functions |
| Impact | Potential ODR violations or compilation errors in unified builds |
How to reproduce
Include WebRTCIPManager.h in another file that uses storageAI Fix Prompt
Fix this issue: WebRTCIPManager `includes` RoverfoxStorageManager.h in cpp but not header. This creates inconsistent compilation units if header is included elsewhere without the required storage header.
Location: patches/webrtc-ip-spoofing.patch (lines 549)
Problem: Header dependency violation - implementation header used only in .cpp
Current behavior: Missing RoverfoxStorageManager.h causes undefined types
Expected: RoverfoxStorageManager.h should be in WebRTCIPManager.h if used in inline functions
Steps to reproduce: Include WebRTCIPManager.h in another file that uses storage
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
| + RoverfoxStorageManager::PutUint(heightKey, static_cast<uint32_t>(aHeight)); | ||
| +} | ||
| + | ||
| +/* static */ bool |
There was a problem hiding this comment.
Bug: ScreenDimensionManager::GetDimensions leaves output parameters uninitialized on failure
ScreenDimensionManager::GetDimensions leaves output parameters uninitialized on failure. When width or height is 0, the function returns false but doesn't set aWidth and aHeight, leaving them with undefined values that callers might use.
View Details
Location: patches/screen-spoofing.patch (lines 48)
Analysis
ScreenDimensionManager::GetDimensions leaves output parameters uninitialized on failure
| What fails | GetDimensions returns false when width/height are 0 but doesn't initialize the output parameters. |
| Result | aWidth and aHeight contain garbage/undefined values when function returns false. |
| Expected | Output parameters should be set to 0 or default values before returning false. |
| Impact | Potential use of uninitialized memory leading to undefined behavior or incorrect screen calculations. |
How to reproduce
Call ScreenDimensionManager::GetDimensions with a context that has no stored dimensions, then attempt to use the width/height values.Patch Details
/* static */ bool
ScreenDimensionManager::GetDimensions(uint32_t aUserContextId, int32_t* aWidth, int32_t* aHeight) {
+ *aWidth = 0;
+ *aHeight = 0;
nsString widthKey, heightKey;AI Fix Prompt
Fix this issue: ScreenDimensionManager::GetDimensions leaves output parameters uninitialized on failure. When width or height is 0, the function returns false but doesn't set aWidth and aHeight, leaving them with undefined values that callers might use.
Location: patches/screen-spoofing.patch (lines 48)
Problem: GetDimensions returns false when width/height are 0 but doesn't initialize the output parameters.
Current behavior: aWidth and aHeight contain garbage/undefined values when function returns false.
Expected: Output parameters should be set to 0 or default values before returning false.
Steps to reproduce: Call ScreenDimensionManager::GetDimensions with a context that has no stored dimensions, then attempt to use the width/height values.
Provide a code fix.
Tip: Reply with @paragon-run to automatically fix this issue
patches/webrtc-ip-spoofing.patch
Outdated
| + if (candStat.mAddress.WasPassed() && !candStat.mAddress.Value().IsEmpty()) { | ||
| + nsAutoCString addr; | ||
| + addr.Assign(NS_ConvertUTF16toUTF8(candStat.mAddress.Value())); | ||
| + bool isIPv6 = addr.FindChar(':') != kNotFound; |
There was a problem hiding this comment.
Is paragon right about the risk of a port falsely setting the ipv6 flag? If so maybe use regex or look for columns AND NOT periods?
2 participants
Summary
This PR resolves Issue daijro#443 and adds critical detection prevention improvements to all fingerprint spoofing patches.
Changes:
Changed Patches
1.
navigator-spoofing.patch(new)Fixes Issue daijro#443 where global CAMOU_CONFIG settings weren't being applied.
What it fixes:
navigator.platformnot spoofednavigator.hardwareConcurrencynot spoofedtimezoneconfig not appliedgeoip=Truecrash (SIGSEGV)Implementation:
Navigator::GetPlatform()andNavigator::HardwareConcurrency()EnsureGlobalTimezoneInitialized()) to avoid SIGSEGV from calling SpiderMonkey before engine is ready2.
webrtc-ip-spoofing.patch(re-enabled and fixed)Re-enables WebRTC IP spoofing with leak prevention.
New additions:
mIceCandidateStats[].mAddressvalues inPeerConnectionImpl::GetStats()callback::1(loopback)fe80::1(link-local)2001:db8::1(documentation)::ffff:192.168.1.1(IPv4-mapped)::(all zeros)3.
timezone-spoofing.patch(improved)Adds timezone persistence across page navigations.
Problem: When a page navigates, a new JS realm is created that doesn't inherit the timezone override. The timezone would revert to default.
Solution: Added hook in
nsGlobalWindowOuter::SetNewDocument():4.
audio-fingerprint-manager.patch(improved)Adds DisableFunction pattern to prevent detection.
Problem:
window.setAudioFingerprintSeedwas permanently visible to page JavaScript, allowing detection.Solution: Dual self-destruct pattern:
JS_DeleteProperty- Removes function from window after first callDisableFunction+IsFunctionEnabledForWebIDL- Prevents function from appearing on new pages in same contextAlso added 3-level
userContextIdresolution (document principal → docshell → top browsing context).5.
screen-spoofing.patch(improved)Adds DisableFunction pattern to both functions:
window.setScreenDimensionswindow.setScreenColorDepthSame self-destruct implementation as audio patch.