Skip to content

Commit ac61109

Browse files
authored
fix: Make SentryBreadcrumb thread-safe to prevent crashes in addBreadcrumb (#7665)
* fix: Make SentryBreadcrumb thread-safe to prevent crashes in addBreadcrumb SentryBreadcrumb properties were nonatomic with no synchronization, and the data property used strong instead of copy. This caused EXC_BAD_ACCESS crashes when serialize read properties concurrently with writes, or when mutable objects nested in data were mutated while sentry_sanitize iterated them. - Add @synchronized(self) to all property accessors and serialize - Deep-copy data in the setter so the SDK never holds references to caller-owned mutable collections - Add defensive [copy] in sentry_sanitize and sanitizeArray before iterating - Replace SENTRY_LOG_DEBUG(@"Add breadcrumb: %@", crumb) with category/message to avoid triggering full serialization on every addBreadcrumb Fixes #2601
1 parent c108237 commit ac61109

7 files changed

Lines changed: 316 additions & 16 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66

77
- Add `SentrySDK.lastRunStatus` to distinguish unknown, no-crash and crash (#7469)
88

9+
### Fixes
10+
11+
- Make SentryBreadcrumb thread-safe to prevent crashes in addBreadcrumb ([#7665](https://github.com/getsentry/sentry-cocoa/pull/7665))
12+
913
## 9.7.0
1014

1115
### Features

Sources/Sentry/Public/SentryBreadcrumb.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ NS_SWIFT_NAME(Breadcrumb)
4949
/**
5050
* Arbitrary additional data that will be sent with the breadcrumb
5151
*/
52-
@property (nonatomic, strong, nullable) NSDictionary<NSString *, id> *data;
52+
@property (nonatomic, copy, nullable) NSDictionary<NSString *, id> *data;
5353

5454
/**
5555
* Initializer for @c SentryBreadcrumb

Sources/Sentry/SentryArray.m

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ @implementation SentryArray
77

88
+ (NSArray *)sanitizeArray:(NSArray *)array;
99
{
10+
// Defensive copy to prevent mutation during enumeration.
11+
NSArray *arrayCopy = [array copy];
12+
1013
NSMutableArray *result = [NSMutableArray array];
11-
for (id rawValue in array) {
14+
for (id rawValue in arrayCopy) {
1215
if ([rawValue isKindOfClass:NSString.class]) {
1316
[result addObject:rawValue];
1417
} else if ([rawValue isKindOfClass:NSNumber.class]) {

Sources/Sentry/SentryBreadcrumb.m

Lines changed: 179 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,154 @@
77
#import "SentryNSDictionarySanitize.h"
88
#import "SentrySwift.h"
99

10+
/**
11+
* Recursively copies a value, producing immutable collections at every level.
12+
* For @c NSDictionary / @c NSArray (including mutable subclasses) a new immutable copy is
13+
* returned with all nested containers copied as well. Other values are returned as-is.
14+
* This prevents the SDK from holding references to caller-owned mutable objects whose
15+
* concurrent mutation would crash serialization (see
16+
* https://github.com/getsentry/sentry-cocoa/issues/2601).
17+
*/
18+
static id
19+
sentry_deepCopyValue(id value)
20+
{
21+
if ([value isKindOfClass:[NSDictionary class]]) {
22+
// Defensive copy to prevent mutation during enumeration.
23+
NSDictionary *dictionaryCopy = [(NSDictionary *)value copy];
24+
NSMutableDictionary *result =
25+
[NSMutableDictionary dictionaryWithCapacity:dictionaryCopy.count];
26+
for (id key in dictionaryCopy) {
27+
id v = dictionaryCopy[key];
28+
if (v != nil) {
29+
result[key] = sentry_deepCopyValue(v);
30+
}
31+
}
32+
return [result copy]; // immutable
33+
} else if ([value isKindOfClass:[NSArray class]]) {
34+
// Defensive copy to prevent mutation during enumeration.
35+
NSArray *arrayCopy = [(NSArray *)value copy];
36+
NSMutableArray *result = [NSMutableArray arrayWithCapacity:arrayCopy.count];
37+
for (id item in arrayCopy) {
38+
[result addObject:sentry_deepCopyValue(item)];
39+
}
40+
return [result copy]; // immutable
41+
}
42+
return value;
43+
}
44+
1045
@implementation SentryBreadcrumb
1146

47+
// Explicit @synthesize so we can provide thread-safe accessors via @synchronized(self).
48+
@synthesize level = _level;
49+
@synthesize category = _category;
50+
@synthesize timestamp = _timestamp;
51+
@synthesize type = _type;
52+
@synthesize message = _message;
53+
@synthesize origin = _origin;
54+
@synthesize data = _data;
55+
56+
#pragma mark - Thread-safe property accessors
57+
58+
- (void)setLevel:(SentryLevel)level
59+
{
60+
@synchronized(self) {
61+
_level = level;
62+
}
63+
}
64+
65+
- (SentryLevel)level
66+
{
67+
@synchronized(self) {
68+
return _level;
69+
}
70+
}
71+
72+
- (void)setCategory:(NSString *)category
73+
{
74+
@synchronized(self) {
75+
_category = [category copy];
76+
}
77+
}
78+
79+
- (NSString *)category
80+
{
81+
@synchronized(self) {
82+
return _category;
83+
}
84+
}
85+
86+
- (void)setTimestamp:(NSDate *)timestamp
87+
{
88+
@synchronized(self) {
89+
_timestamp = timestamp;
90+
}
91+
}
92+
93+
- (NSDate *)timestamp
94+
{
95+
@synchronized(self) {
96+
return _timestamp;
97+
}
98+
}
99+
100+
- (void)setType:(NSString *)type
101+
{
102+
@synchronized(self) {
103+
_type = [type copy];
104+
}
105+
}
106+
107+
- (NSString *)type
108+
{
109+
@synchronized(self) {
110+
return _type;
111+
}
112+
}
113+
114+
- (void)setMessage:(NSString *)message
115+
{
116+
@synchronized(self) {
117+
_message = [message copy];
118+
}
119+
}
120+
121+
- (NSString *)message
122+
{
123+
@synchronized(self) {
124+
return _message;
125+
}
126+
}
127+
128+
- (void)setOrigin:(NSString *)origin
129+
{
130+
@synchronized(self) {
131+
_origin = [origin copy];
132+
}
133+
}
134+
135+
- (NSString *)origin
136+
{
137+
@synchronized(self) {
138+
return _origin;
139+
}
140+
}
141+
142+
- (void)setData:(NSDictionary<NSString *, id> *)data
143+
{
144+
@synchronized(self) {
145+
_data = data ? sentry_deepCopyValue(data) : nil;
146+
}
147+
}
148+
149+
- (NSDictionary<NSString *, id> *)data
150+
{
151+
@synchronized(self) {
152+
return _data;
153+
}
154+
}
155+
156+
#pragma mark - Initializers
157+
12158
- (instancetype)initWithDictionary:(NSDictionary *)dictionary
13159
{
14160
if (self = [super init]) {
@@ -56,24 +202,46 @@ - (instancetype)init
56202
return [self initWithLevel:kSentryLevelInfo category:@"default"];
57203
}
58204

205+
#pragma mark - Serialization
206+
59207
- (NSDictionary<NSString *, id> *)serialize
60208
{
209+
// Capture all properties under the lock to get a consistent snapshot.
210+
SentryLevel level;
211+
NSDate *timestamp;
212+
NSString *category;
213+
NSString *type;
214+
NSString *origin;
215+
NSString *message;
216+
NSDictionary *data;
217+
218+
@synchronized(self) {
219+
level = _level;
220+
timestamp = _timestamp;
221+
category = _category;
222+
type = _type;
223+
origin = _origin;
224+
message = _message;
225+
data = _data;
226+
}
227+
61228
NSMutableDictionary *serializedData = [[NSMutableDictionary alloc] init];
62229

63-
[serializedData setValue:nameForSentryLevel(self.level) forKey:@"level"];
64-
if (self.timestamp != nil) {
65-
[serializedData
66-
setValue:sentry_toIso8601String(SENTRY_UNWRAP_NULLABLE(NSDate, self.timestamp))
67-
forKey:@"timestamp"];
68-
}
69-
[serializedData setValue:self.category forKey:@"category"];
70-
[serializedData setValue:self.type forKey:@"type"];
71-
[serializedData setValue:self.origin forKey:@"origin"];
72-
[serializedData setValue:self.message forKey:@"message"];
73-
[serializedData setValue:sentry_sanitize(self.data) forKey:@"data"];
230+
[serializedData setValue:nameForSentryLevel(level) forKey:@"level"];
231+
if (timestamp != nil) {
232+
[serializedData setValue:sentry_toIso8601String(SENTRY_UNWRAP_NULLABLE(NSDate, timestamp))
233+
forKey:@"timestamp"];
234+
}
235+
[serializedData setValue:category forKey:@"category"];
236+
[serializedData setValue:type forKey:@"type"];
237+
[serializedData setValue:origin forKey:@"origin"];
238+
[serializedData setValue:message forKey:@"message"];
239+
[serializedData setValue:sentry_sanitize(data) forKey:@"data"];
74240
return serializedData;
75241
}
76242

243+
#pragma mark - Equality
244+
77245
- (BOOL)isEqual:(id _Nullable)other
78246
{
79247
if (other == self)

Sources/Sentry/SentryNSDictionarySanitize.m

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@
1212
return nil;
1313
}
1414

15+
// Defensive copy to prevent mutation during enumeration.
16+
NSDictionary *dictionaryCopy = [dictionary copy];
17+
1518
NSMutableDictionary *dict = [NSMutableDictionary dictionary];
16-
for (id rawKey in dictionary.allKeys) {
17-
id rawValue = [dictionary objectForKey:rawKey];
19+
for (id rawKey in dictionaryCopy.allKeys) {
20+
id rawValue = [dictionaryCopy objectForKey:rawKey];
1821

1922
NSString *stringKey;
2023
if ([rawKey isKindOfClass:NSString.class]) {

Sources/Sentry/SentryScope.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ - (void)addBreadcrumb:(SentryBreadcrumb *)crumb
102102
if (self.maxBreadcrumbs < 1) {
103103
return;
104104
}
105-
SENTRY_LOG_DEBUG(@"Add breadcrumb: %@", crumb);
105+
SENTRY_LOG_DEBUG(@"Add breadcrumb: %@ - %@", crumb.category, crumb.message);
106106
@synchronized(_breadcrumbArray) {
107107
// Use a ring buffer making adding breadcrumbs O(1).
108108
// In a prior version, we added the new breadcrumb at the end of the array and used

0 commit comments

Comments
 (0)