Skip to content

Commit

Permalink
DispatchTimeInterval & Date: avoid 32-bit overflows, fix `watchOS…
Browse files Browse the repository at this point in the history
…` crashes (#2342)

Fixes #2338, [SDKONCALL-237], [SDK-2974].

`Int` is actually 32 bits in 32-bit platforms, like `watchOS` still is
(sort of).
This fixes 2 potential `Int` overflows: `Date.millisecondsSince1970` and
`DispatchTimeInterval` arithmetic using `nanoseconds`.

See also #2340.

[SDKONCALL-237]:
https://revenuecats.atlassian.net/browse/SDKONCALL-237?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
NachoSoto committed Mar 16, 2023
1 parent 2fff5b0 commit d4ecc7b
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 40 deletions.
12 changes: 8 additions & 4 deletions RevenueCat.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@
351B519F26D4508A00BD2BD7 /* DeviceCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E35D87B7E6F91E27E98F42 /* DeviceCacheTests.swift */; };
351B51A326D450BC00BD2BD7 /* DictionaryExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DD269162522A20A006AC4BC /* DictionaryExtensionsTests.swift */; };
351B51A426D450BC00BD2BD7 /* NSError+RCExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E353AF2CAD3CEDE6D9B368 /* NSError+RCExtensionsTests.swift */; };
351B51A526D450BC00BD2BD7 /* NSDate+RCExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E3508EC20EEBAB4EAC4C82 /* NSDate+RCExtensionsTests.swift */; };
351B51A526D450BC00BD2BD7 /* DateExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E3508EC20EEBAB4EAC4C82 /* DateExtensionsTests.swift */; };
351B51A726D450D400BD2BD7 /* SystemInfoTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E35EEE7783629CDE41B70C /* SystemInfoTests.swift */; };
351B51B526D450E800BD2BD7 /* ProductsFetcherSK1Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37E351F0E21361EAEC078A0D /* ProductsFetcherSK1Tests.swift */; };
351B51B626D450E800BD2BD7 /* ReceiptFetcherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D84458826B9CD270033B5A3 /* ReceiptFetcherTests.swift */; };
Expand Down Expand Up @@ -300,6 +300,7 @@
578DAA482948EEAD001700FD /* Clock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 578DAA472948EEAD001700FD /* Clock.swift */; };
578DAA4A2948EF4F001700FD /* TestClock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 578DAA492948EF4F001700FD /* TestClock.swift */; };
578FB10E27ADDA8000F70709 /* AvailabilityChecks.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3BE0263275942D500915B4C /* AvailabilityChecks.swift */; };
57910CB329C3889B006209D5 /* DispatchTimeIntervalExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 57910CB229C3889B006209D5 /* DispatchTimeIntervalExtensionsTests.swift */; };
579189B728F4747700BF4963 /* EitherTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 579189B628F4747700BF4963 /* EitherTests.swift */; };
579189E928F47E8D00BF4963 /* PurchasesDiagnosticsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 579189E828F47E8D00BF4963 /* PurchasesDiagnosticsTests.swift */; };
579189EB28F47F0F00BF4963 /* MockPurchases.swift in Sources */ = {isa = PBXBuildFile; fileRef = 579189EA28F47F0F00BF4963 /* MockPurchases.swift */; };
Expand Down Expand Up @@ -779,7 +780,7 @@
37E350420D54B99BB39448E0 /* AttributionTypeFactoryTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AttributionTypeFactoryTests.swift; sourceTree = "<group>"; };
37E3507939634ED5A9280544 /* Strings.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Strings.swift; sourceTree = "<group>"; };
37E3508E52201122137D4B4A /* PurchasesSubscriberAttributesTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PurchasesSubscriberAttributesTests.swift; sourceTree = "<group>"; };
37E3508EC20EEBAB4EAC4C82 /* NSDate+RCExtensionsTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "NSDate+RCExtensionsTests.swift"; sourceTree = "<group>"; };
37E3508EC20EEBAB4EAC4C82 /* DateExtensionsTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DateExtensionsTests.swift; sourceTree = "<group>"; };
37E35092F0E41512E0D610BA /* ContainerFactory.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ContainerFactory.swift; sourceTree = "<group>"; };
37E350E57B0A393455A72B40 /* ProductRequestDataTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProductRequestDataTests.swift; sourceTree = "<group>"; };
37E351D0EBC4698E1D3585A6 /* ReceiptParserTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReceiptParserTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -916,6 +917,7 @@
578D79932936B0810042E434 /* LoggerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoggerTests.swift; sourceTree = "<group>"; };
578DAA472948EEAD001700FD /* Clock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Clock.swift; sourceTree = "<group>"; };
578DAA492948EF4F001700FD /* TestClock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestClock.swift; sourceTree = "<group>"; };
57910CB229C3889B006209D5 /* DispatchTimeIntervalExtensionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DispatchTimeIntervalExtensionsTests.swift; sourceTree = "<group>"; };
57910CBD29C393A6006209D5 /* CI-RevenueCat.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; name = "CI-RevenueCat.xctestplan"; path = "Tests/TestPlans/CI-RevenueCat.xctestplan"; sourceTree = "<group>"; };
579189B628F4747700BF4963 /* EitherTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EitherTests.swift; sourceTree = "<group>"; };
579189E828F47E8D00BF4963 /* PurchasesDiagnosticsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PurchasesDiagnosticsTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1837,11 +1839,12 @@
children = (
572247F627BF1ADF00C524A7 /* ArrayExtensionsTests.swift */,
57069A5D28E398E100B86355 /* AsyncExtensionsTests.swift */,
37E3508EC20EEBAB4EAC4C82 /* DateExtensionsTests.swift */,
37E35FDA0A44EA03EA12DAA2 /* DateFormatter+ExtensionsTests.swift */,
57ACB13628184CF1000DCC9F /* DecoderExtensionTests.swift */,
2DD269162522A20A006AC4BC /* DictionaryExtensionsTests.swift */,
57910CB229C3889B006209D5 /* DispatchTimeIntervalExtensionsTests.swift */,
37E35ABEE9FD79CCA64E4F8B /* NSData+RCExtensionsTests.swift */,
37E3508EC20EEBAB4EAC4C82 /* NSDate+RCExtensionsTests.swift */,
37E353AF2CAD3CEDE6D9B368 /* NSError+RCExtensionsTests.swift */,
5766AA41283C768600FA6091 /* OperatorExtensionsTests.swift */,
5759B321296DEF56002472D5 /* OptionalExtensionsTests.swift */,
Expand Down Expand Up @@ -3042,6 +3045,7 @@
573A10D52800A7C800F976E5 /* SKErrorTests.swift in Sources */,
B31C8BEC285BD58F001017B7 /* MockIdentityAPI.swift in Sources */,
351B513D26D4491E00BD2BD7 /* MockDeviceCache.swift in Sources */,
57910CB329C3889B006209D5 /* DispatchTimeIntervalExtensionsTests.swift in Sources */,
351B515C26D44B7900BD2BD7 /* MockIntroEligibilityCalculator.swift in Sources */,
5733D01128D00354008638D8 /* PromotionalOfferTests.swift in Sources */,
57544C28285FA94B004E54D5 /* MockAttributeSyncing.swift in Sources */,
Expand Down Expand Up @@ -3114,7 +3118,7 @@
573A10D92800ADCD00F976E5 /* StoreKitErrorTests.swift in Sources */,
5766AABF283E80B500FA6091 /* BasePurchasesTests.swift in Sources */,
351B515826D44B3E00BD2BD7 /* MockOfferingsFactory.swift in Sources */,
351B51A526D450BC00BD2BD7 /* NSDate+RCExtensionsTests.swift in Sources */,
351B51A526D450BC00BD2BD7 /* DateExtensionsTests.swift in Sources */,
B390F5BA271DDC7400B64D65 /* PurchasesDeprecation.swift in Sources */,
57FDAABE28493A29009A48F1 /* SandboxEnvironmentDetectorTests.swift in Sources */,
57BA943128330ACA00CD5FC5 /* ConfigurationTests.swift in Sources */,
Expand Down
19 changes: 4 additions & 15 deletions Sources/FoundationExtensions/Date+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,15 @@

import Foundation

extension NSDate {

func millisecondsSince1970AsUInt64() -> UInt64 {
return (self as Date).millisecondsSince1970AsUInt64()
}

}

extension Date {

init(millisecondsSince1970: Int) {
init(millisecondsSince1970: UInt64) {
self.init(timeIntervalSince1970: TimeInterval(millisecondsSince1970) / 1000)
}

var millisecondsSince1970: Int {
return Int(self.timeIntervalSince1970 * 1000.0)
}

func millisecondsSince1970AsUInt64() -> UInt64 {
return UInt64(self.millisecondsSince1970)
/// - Important: this needs to be 64 bits because `Int` is 32 bits in watchOS
var millisecondsSince1970: UInt64 {
return UInt64(self.timeIntervalSince1970 * 1000)
}

}
33 changes: 25 additions & 8 deletions Sources/FoundationExtensions/DispatchTimeInterval+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,25 @@ extension DispatchTimeInterval {

/// `DispatchTimeInterval` can only be used by specifying a unit of time.
/// This allows us to easily convert any `DispatchTimeInterval` into nanoseconds.
var nanoseconds: Int {
/// - Important: It's likely that `x * 1_000_000_000` can't be represented in 32 bits.
var nanoseconds: UInt64 {
switch self {
case let .seconds(s): return s * 1_000_000_000
case let .milliseconds(ms): return ms * 1_000_000
case let .microseconds(ms): return ms * 1000
case let .nanoseconds(ns): return ns
case let .seconds(s): return UInt64(s) * UInt64(1_000_000_000)
case let .milliseconds(ms): return UInt64(ms) * UInt64(1_000_000)
case let .microseconds(ms): return UInt64(ms) * UInt64(1000)
case let .nanoseconds(ns): return UInt64(ns)
case .never: return 0
@unknown default: fatalError("Unknown value: \(self)")
}
}

/// - Note: this returns `Int`, so it might lose precision for `.milliseconds` and `.microseconds`.
var milliseconds: Int {
switch self {
case let .seconds(s): return s * 1_000
case let .milliseconds(ms): return ms
case let .microseconds(ms): return Int((Double(ms) / 1_000).rounded())
case let .nanoseconds(ns): return Int((Double(ns) / 1_000_000).rounded())
case .never: return 0
@unknown default: fatalError("Unknown value: \(self)")
}
Expand All @@ -45,7 +58,7 @@ extension DispatchTimeInterval {
var seconds: Double {
switch self {
case let .seconds(seconds): return Double(seconds)
case let .milliseconds(ms): return Double(ms) / 1000
case let .milliseconds(ms): return Double(ms) / 1_000
case let .microseconds(ms): return Double(ms) / 1_000_000
case let .nanoseconds(ns): return Double(ns) / 1_000_000_000
case .never: return 0
Expand All @@ -62,11 +75,15 @@ extension DispatchTimeInterval {
// swiftlint:enable identifier_name

func + (lhs: DispatchTimeInterval, rhs: DispatchTimeInterval) -> DispatchTimeInterval {
return .nanoseconds(lhs.nanoseconds + rhs.nanoseconds)
// Note: `DispatchTimeInterval` uses `Int` for nanoseconds, which might overflow in 32 bits
// This loses some precision by using milliseconds, but avoids potential overflows.
return .milliseconds(lhs.milliseconds + rhs.milliseconds)
}

func - (lhs: DispatchTimeInterval, rhs: DispatchTimeInterval) -> DispatchTimeInterval {
return .nanoseconds(lhs.nanoseconds - rhs.nanoseconds)
// Note: `DispatchTimeInterval` uses `Int` for nanoseconds, which might overflow in 32 bits
// This loses some precision by using milliseconds, but avoids potential overflows.
return .milliseconds(lhs.milliseconds - rhs.milliseconds)
}

extension DispatchTimeInterval: Comparable {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Security/Signing+ResponseVerification.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ extension HTTPResponse {
forCaseInsensitiveHeaderField: HTTPClient.ResponseHeader.requestDate.rawValue,
in: headers
),
let intValue = Int(stringValue) else { return nil }
let intValue = UInt64(stringValue) else { return nil }

return .init(millisecondsSince1970: intValue)
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Security/Signing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ enum Signing: SigningType {

let message: Data
let nonce: Data
let requestDate: Int
let requestDate: UInt64

}

Expand Down
2 changes: 1 addition & 1 deletion Sources/SubscriberAttributes/SubscriberAttribute.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ extension SubscriberAttribute {

func asBackendDictionary() -> [String: Any] {
return [BackendKey.value.rawValue: self.value,
BackendKey.timestamp.rawValue: self.setTime.millisecondsSince1970AsUInt64()]
BackendKey.timestamp.rawValue: self.setTime.millisecondsSince1970]
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@ import XCTest

@testable import RevenueCat

class NSDateExtensionsTests: TestCase {
class DateExtensionsTests: TestCase {

func testMillisecondsSince1970ConvertsCorrectlyWithCurrentTime() {
let date = NSDate()
expect(date.millisecondsSince1970AsUInt64()) == (UInt64)(date.timeIntervalSince1970 * 1000)
let date = Date()
expect(date.millisecondsSince1970) == UInt64(date.timeIntervalSince1970 * 1000)
}

func testMillisecondsSince1970ConvertsCorrectlyWithFixedTime() {
let secondsSince1970: TimeInterval = 1619555571.0
let millisecondsSince1970UInt64: UInt64 = 1619555571000
let date = NSDate(timeIntervalSince1970: secondsSince1970)
expect(date.millisecondsSince1970AsUInt64()) == millisecondsSince1970UInt64
let date = Date(timeIntervalSince1970: secondsSince1970)
expect(date.millisecondsSince1970) == millisecondsSince1970UInt64
}

func testMillisecondsSince1970() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//
// Copyright RevenueCat Inc. All Rights Reserved.
//
// Licensed under the MIT License (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://opensource.org/licenses/MIT
//
// DispatchTimeIntervalExtensionsTests.swift
//
// Created by Nacho Soto on 3/16/23.

import Nimble
import XCTest

@testable import RevenueCat

// swiftlint:disable identifier_name

class DispatchTimeIntervalExtensionsTests: TestCase {

func testIsLessThan() {
let a: DispatchTimeInterval = .seconds(1)
let b: DispatchTimeInterval = .seconds(2)

expect(a) < b
}

func testIsGreaterThan() {
let a: DispatchTimeInterval = .seconds(2)
let b: DispatchTimeInterval = .seconds(1)

expect(a) > b
}

func testSecondsToMilliseconds() {
expect(DispatchTimeInterval.seconds(5).milliseconds) == 5_000
}

func testSecondsToNanoseconds() {
expect(DispatchTimeInterval.seconds(5).nanoseconds) == 5_000_000_000
}

func testAddingSeconds() {
let a: DispatchTimeInterval = .seconds(2)
let b: DispatchTimeInterval = .seconds(1)

expect(a + b) == .seconds(3)
}

func testAddingMilliseconds() {
let a: DispatchTimeInterval = .milliseconds(2)
let b: DispatchTimeInterval = .milliseconds(1)

expect(a + b) == .milliseconds(3)
}

func testAddingNanoseconds() {
let a: DispatchTimeInterval = .nanoseconds(2_000_000)
let b: DispatchTimeInterval = .nanoseconds(1_000_000)

expect(a + b) == .milliseconds(3)
}

func testSubstractingSeconds() {
let a: DispatchTimeInterval = .seconds(3)
let b: DispatchTimeInterval = .seconds(1)

expect(a - b) == .seconds(2)
}

func testSubstractingMilliseconds() {
let a: DispatchTimeInterval = .milliseconds(3)
let b: DispatchTimeInterval = .milliseconds(1)

expect(a - b) == .milliseconds(2)
}

func testSubstractingNanoseconds() {
let a: DispatchTimeInterval = .nanoseconds(3_000_000)
let b: DispatchTimeInterval = .nanoseconds(1_000_000)

expect(a - b) == .milliseconds(2)
}

}
8 changes: 4 additions & 4 deletions Tests/UnitTests/Security/SigningTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class SigningTests: TestCase {

let message = "Hello World"
let nonce = "nonce"
let requestDate = 1677005916012
let requestDate: UInt64 = 1677005916012
let signature = "this is not a signature"

expect(Signing.verify(
Expand Down Expand Up @@ -113,7 +113,7 @@ class SigningTests: TestCase {
func testVerifySignatureWithValidSignature() throws {
let message = "Hello World"
let nonce = "nonce"
let requestDate = 1677005916012
let requestDate: UInt64 = 1677005916012
let salt = Self.createSalt()

let signature = try self.sign(
Expand Down Expand Up @@ -155,7 +155,7 @@ class SigningTests: TestCase {
// swiftlint:enable line_length

let nonce = try XCTUnwrap(Data(base64Encoded: "MTIzNDU2Nzg5MGFi"))
let requestDate = 1677005916012
let requestDate: UInt64 = 1677005916012

expect(
Signing.verify(
Expand All @@ -182,7 +182,7 @@ class SigningTests: TestCase {
*/

let nonce = try XCTUnwrap(Data(base64Encoded: "MTIzNDU2Nzg5MGFi"))
let requestDate = 1677013582768
let requestDate: UInt64 = 1677013582768
let eTag = "b7bd9a697c7fd1a2"

// swiftlint:disable:next line_length
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class SubscriberAttributeTests: TestCase {

expect(receivedDictionary["value"] as? String) == Self.mockAttribute.value
expect((receivedDictionary["updated_at_ms"] as? NSNumber)?.uint64Value)
== Self.mockAttribute.setTime.millisecondsSince1970AsUInt64()
== Self.mockAttribute.setTime.millisecondsSince1970
}

func testInitWithDictionarySetsRightValues() throws {
Expand Down

0 comments on commit d4ecc7b

Please sign in to comment.