[vendored] RRConfig.h state-mutating accessors are not atomic and not retry-safe under concurrent sessions

[dariusta]

Source

Filed from a downstream consumer (Straton-Labs-LLC/stratton-internal, vendoring WebDriverAgentLib.xcframework).
Tracking ID in our audit: ocr-prelim-04296.

Affected file

WebDriverAgentLib/RodmanRunnerLib/RRConfig.h (vendored header at native/runner/Frameworks/WebDriverAgentLib.xcframework/ios-arm64/RodmanRunnerLib.framework/Headers/RRConfig.h)

Symptom

The class exposes ~30 pairs of + (void)setX:(T)value; / + (T)x; accessors backed by static globals (e.g. setShouldUseCompactResponses:, setMaxTypingFrequency:, setWaitForIdleTimeout:, setScreenshotOrientation:error:). Each pair is a plain read or write to a global with no synchronization.

Representative lines:

+ (void)setShouldUseCompactResponses:(BOOL)value;
+ (BOOL)shouldUseCompactResponses;

+ (void)setMaxTypingFrequency:(NSUInteger)value;
+ (NSUInteger)maxTypingFrequency;

+ (void)setWaitForIdleTimeout:(NSTimeInterval)timeout;
+ (NSTimeInterval)waitForIdleTimeout;

+ (BOOL)setScreenshotOrientation:(NSString *)orientation error:(NSError **)error;
+ (NSInteger)screenshotOrientation;

Defect

• TOCTOU on reads. bool* readSnapshotConfig = [RRConfig shouldUseCompactResponses]; ... ; [RRConfig setShouldUseCompactResponses:NO]; is not atomic; if another test target thread (XCTest runs test methods on a private queue) sets a different value between the read and the write, the caller has a stale value with no diagnostic.
• Write torn reads. On architectures where the global is wider than the natural word size (NSUInteger is unsigned long; NSTimeInterval is double), the read can observe a partially-written value. We have observed this in CI on a 32-bit slice for the typing-frequency field.
• No retry / idempotency contract. The mutating side does not document "last writer wins" vs. "session-scoped" semantics. A second setMaxTypingFrequency: call from a cleanup hook can clobber the value the next test was relying on, and there is no way to detect or recover.

Suggested fix

Two changes, both small:

1. Serialize the accessors. Either wrap every pair with @synchronized(self) (cheap; the call sites are not hot paths) or move the globals into an os_unfair_lock-guarded struct.
2. Document the idempotency contract in each setX: doc-comment. The minimum useful statement is: "This setting is global and persists across sessions within the process lifetime. Callers must serialize calls to setX: and x from the same thread that owns the runner session, or hold an external lock."

For setScreenshotOrientation:error: in particular, return NSError * for the rejected-argument case (currently the only failure path), so the caller can fall back to "auto" without silent partial state.

Why this matters downstream

A runner that re-vends WDA each release inherits whatever locking the upstream header documents. We patch locally as a stop-gap; re-vendoring would clobber that.