Skip to content

Commit a69a51f

Browse files
buenaflorbuenaflor
authored
Fix unsafe json access in sentry_device (#3309)
* Add type safe access extension to Map<String, dynamic> * Add tests * Update CHANGELOG * Fix analyze * getBool should be able to handle numerical values 1 and 0 * UpdatE * Update
1 parent 393f8ec commit a69a51f

File tree

5 files changed

+457
-46
lines changed

5 files changed

+457
-46
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
- Offload `captureEnvelope` to background isolate for Cocoa and Android ([#3232](https://github.com/getsentry/sentry-dart/pull/3232))
1818
- Add `sentry.replay_id` to flutter logs ([#3257](https://github.com/getsentry/sentry-dart/pull/3257))
1919

20+
### Fixes
21+
22+
- Fix unsafe json access in `sentry_device` ([#3309](https://github.com/getsentry/sentry-dart/pull/3309))
23+
2024
## 9.7.0
2125

2226
### Features

‎packages/dart/lib/src/protocol/sentry_device.dart‎

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'package:meta/meta.dart';
22
import '../sentry_options.dart';
3+
import '../utils/type_safe_map_access.dart';
34
import 'access_aware_map.dart';
45

56
/// If a device is on portrait or landscape mode
@@ -179,52 +180,46 @@ class SentryDevice {
179180
factory SentryDevice.fromJson(Map<String, dynamic> data) {
180181
final json = AccessAwareMap(data);
181182
return SentryDevice(
182-
name: json['name'],
183-
family: json['family'],
184-
model: json['model'],
185-
modelId: json['model_id'],
186-
arch: json['arch'],
187-
batteryLevel:
188-
(json['battery_level'] is num ? json['battery_level'] as num : null)
189-
?.toDouble(),
190-
orientation: json['orientation'] == 'portrait'
191-
? SentryOrientation.portrait
192-
: json['orientation'] == 'landscape'
193-
? SentryOrientation.landscape
194-
: null,
195-
manufacturer: json['manufacturer'],
196-
brand: json['brand'],
197-
screenHeightPixels: json['screen_height_pixels']?.toInt(),
198-
screenWidthPixels: json['screen_width_pixels']?.toInt(),
199-
screenDensity: json['screen_density'],
200-
screenDpi: json['screen_dpi'],
201-
online: json['online'],
202-
charging: json['charging'],
203-
lowMemory: json['low_memory'],
204-
simulator: json['simulator'],
205-
memorySize: json['memory_size'],
206-
freeMemory: json['free_memory'],
207-
usableMemory: json['usable_memory'],
208-
storageSize: json['storage_size'],
209-
freeStorage: json['free_storage'],
210-
externalStorageSize: json['external_storage_size'],
211-
externalFreeStorage: json['external_free_storage'],
212-
bootTime: json['boot_time'] != null
213-
? DateTime.tryParse(json['boot_time'])
214-
: null,
215-
processorCount: json['processor_count'],
216-
cpuDescription: json['cpu_description'],
217-
processorFrequency: (json['processor_frequency'] is num)
218-
? (json['processor_frequency'] as num).toDouble()
219-
: null,
220-
deviceType: json['device_type'],
221-
batteryStatus: json['battery_status'],
222-
deviceUniqueIdentifier: json['device_unique_identifier'],
223-
supportsVibration: json['supports_vibration'],
224-
supportsAccelerometer: json['supports_accelerometer'],
225-
supportsGyroscope: json['supports_gyroscope'],
226-
supportsAudio: json['supports_audio'],
227-
supportsLocationService: json['supports_location_service'],
183+
name: json.getValueOrNull('name'),
184+
family: json.getValueOrNull('family'),
185+
model: json.getValueOrNull('model'),
186+
modelId: json.getValueOrNull('model_id'),
187+
arch: json.getValueOrNull('arch'),
188+
batteryLevel: json.getValueOrNull('battery_level'),
189+
orientation: switch (json.getValueOrNull('orientation')) {
190+
'portrait' => SentryOrientation.portrait,
191+
'landscape' => SentryOrientation.landscape,
192+
_ => null,
193+
},
194+
manufacturer: json.getValueOrNull('manufacturer'),
195+
brand: json.getValueOrNull('brand'),
196+
screenHeightPixels: json.getValueOrNull('screen_height_pixels'),
197+
screenWidthPixels: json.getValueOrNull('screen_width_pixels'),
198+
screenDensity: json.getValueOrNull('screen_density'),
199+
screenDpi: json.getValueOrNull('screen_dpi'),
200+
online: json.getValueOrNull('online'),
201+
charging: json.getValueOrNull('charging'),
202+
lowMemory: json.getValueOrNull('low_memory'),
203+
simulator: json.getValueOrNull('simulator'),
204+
memorySize: json.getValueOrNull('memory_size'),
205+
freeMemory: json.getValueOrNull('free_memory'),
206+
usableMemory: json.getValueOrNull('usable_memory'),
207+
storageSize: json.getValueOrNull('storage_size'),
208+
freeStorage: json.getValueOrNull('free_storage'),
209+
externalStorageSize: json.getValueOrNull('external_storage_size'),
210+
externalFreeStorage: json.getValueOrNull('external_free_storage'),
211+
bootTime: json.getValueOrNull('boot_time'),
212+
processorCount: json.getValueOrNull('processor_count'),
213+
cpuDescription: json.getValueOrNull('cpu_description'),
214+
processorFrequency: json.getValueOrNull('processor_frequency'),
215+
deviceType: json.getValueOrNull('device_type'),
216+
batteryStatus: json.getValueOrNull('battery_status'),
217+
deviceUniqueIdentifier: json.getValueOrNull('device_unique_identifier'),
218+
supportsVibration: json.getValueOrNull('supports_vibration'),
219+
supportsAccelerometer: json.getValueOrNull('supports_accelerometer'),
220+
supportsGyroscope: json.getValueOrNull('supports_gyroscope'),
221+
supportsAudio: json.getValueOrNull('supports_audio'),
222+
supportsLocationService: json.getValueOrNull('supports_location_service'),
228223
unknown: json.notAccessed(),
229224
);
230225
}

‎packages/dart/lib/src/utils/type_safe_map_access.dart‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import 'package:meta/meta.dart';
2+
import '../sentry.dart';
3+
import '../protocol/sentry_level.dart';
4+
5+
/// Extension providing type-safe value extraction from JSON maps
6+
@internal
7+
extension TypeSafeMapExtension on Map<String, dynamic> {
8+
/// Generic, type-safe extraction with a few built-in coercions:
9+
/// - num -> int
10+
/// - num -> double
11+
/// - 0/1 -> bool
12+
/// - String (ISO) -> DateTime
13+
T? getValueOrNull<T>(String key) {
14+
final value = this[key];
15+
if (value == null) return null;
16+
17+
final convertedValue = _tryConvertValue<T>(key, value);
18+
if (convertedValue != null) return convertedValue;
19+
20+
_logTypeMismatch(key, _expectedTypeFor<T>(), value.runtimeType.toString());
21+
return null;
22+
}
23+
24+
T? _tryConvertValue<T>(String key, Object value) {
25+
// Direct hit.
26+
if (value is T) return value as T;
27+
28+
// num -> int
29+
if (T == int) {
30+
if (value is num) return value.toInt() as T;
31+
return null;
32+
}
33+
34+
// num -> double
35+
if (T == double) {
36+
if (value is num) return value.toDouble() as T;
37+
return null;
38+
}
39+
40+
// 0/1 -> bool
41+
if (T == bool) {
42+
// if value is bool directly already handled above
43+
if (value is num) {
44+
if (value == 0) return false as T;
45+
if (value == 1) return true as T;
46+
}
47+
return null;
48+
}
49+
50+
// String(ISO8601) -> DateTime
51+
if (T == DateTime) {
52+
if (value is! String) {
53+
_logTypeMismatch(
54+
key, 'String (for DateTime)', value.runtimeType.toString());
55+
return null;
56+
}
57+
final dt = DateTime.tryParse(value);
58+
if (dt == null) {
59+
_logParseError(key, 'DateTime', value);
60+
return null;
61+
}
62+
return dt as T;
63+
}
64+
65+
return null;
66+
}
67+
68+
String _expectedTypeFor<T>() {
69+
if (T == DateTime) {
70+
return 'String (for DateTime)';
71+
}
72+
return T.toString();
73+
}
74+
75+
void _logTypeMismatch(String key, String expected, String actual) {
76+
Sentry.currentHub.options.log(
77+
SentryLevel.warning,
78+
'Type mismatch in JSON deserialization: key "$key" expected $expected but got $actual',
79+
);
80+
}
81+
82+
void _logParseError(String key, String expected, Object value) {
83+
Sentry.currentHub.options.log(
84+
SentryLevel.warning,
85+
'Parse error in JSON deserialization: key "$key" could not be parsed as $expected from value "$value"',
86+
);
87+
}
88+
}

‎packages/dart/test/protocol/sentry_device_test.dart‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,154 @@ void main() {
152152
null,
153153
);
154154
});
155+
156+
test('orientation handles portrait', () {
157+
final map = {'orientation': 'portrait'};
158+
final sentryDevice = SentryDevice.fromJson(map);
159+
expect(sentryDevice.orientation, SentryOrientation.portrait);
160+
});
161+
162+
test('orientation handles landscape', () {
163+
final map = {'orientation': 'landscape'};
164+
final sentryDevice = SentryDevice.fromJson(map);
165+
expect(sentryDevice.orientation, SentryOrientation.landscape);
166+
});
167+
168+
test('orientation returns null for invalid enum value', () {
169+
final map = {'orientation': 'invalid'};
170+
final sentryDevice = SentryDevice.fromJson(map);
171+
expect(sentryDevice.orientation, isNull);
172+
});
173+
174+
test('orientation returns null for non-string value', () {
175+
final map = {'orientation': 123};
176+
final sentryDevice = SentryDevice.fromJson(map);
177+
expect(sentryDevice.orientation, isNull);
178+
});
179+
180+
test('bootTime parses valid ISO8601 string', () {
181+
final dateTime = DateTime(2023, 10, 15, 12, 30, 45);
182+
final map = {'boot_time': dateTime.toIso8601String()};
183+
final sentryDevice = SentryDevice.fromJson(map);
184+
expect(sentryDevice.bootTime, isNotNull);
185+
expect(sentryDevice.bootTime!.year, 2023);
186+
expect(sentryDevice.bootTime!.month, 10);
187+
expect(sentryDevice.bootTime!.day, 15);
188+
});
189+
190+
test('bootTime returns null for invalid date string', () {
191+
final map = {'boot_time': 'not a date'};
192+
final sentryDevice = SentryDevice.fromJson(map);
193+
expect(sentryDevice.bootTime, isNull);
194+
});
195+
196+
test('bootTime returns null for non-string value', () {
197+
final map = {'boot_time': 12345};
198+
final sentryDevice = SentryDevice.fromJson(map);
199+
expect(sentryDevice.bootTime, isNull);
200+
});
201+
202+
test('string fields return null for non-string values', () {
203+
final map = {
204+
'name': 123,
205+
'family': true,
206+
'model': ['array'],
207+
'arch': {'object': 'value'},
208+
};
209+
final sentryDevice = SentryDevice.fromJson(map);
210+
expect(sentryDevice.name, isNull);
211+
expect(sentryDevice.family, isNull);
212+
expect(sentryDevice.model, isNull);
213+
expect(sentryDevice.arch, isNull);
214+
});
215+
216+
test('int fields return null for non-numeric values', () {
217+
final map = {
218+
'screen_height_pixels': 'not a number',
219+
'screen_width_pixels': true,
220+
'screen_dpi': ['array'],
221+
'processor_count': {'object': 'value'},
222+
};
223+
final sentryDevice = SentryDevice.fromJson(map);
224+
expect(sentryDevice.screenHeightPixels, isNull);
225+
expect(sentryDevice.screenWidthPixels, isNull);
226+
expect(sentryDevice.screenDpi, isNull);
227+
expect(sentryDevice.processorCount, isNull);
228+
});
229+
230+
test('double fields return null for non-numeric values', () {
231+
final map = {
232+
'screen_density': 'not a number',
233+
'processor_frequency': true,
234+
};
235+
final sentryDevice = SentryDevice.fromJson(map);
236+
expect(sentryDevice.screenDensity, isNull);
237+
expect(sentryDevice.processorFrequency, isNull);
238+
});
239+
240+
test('bool fields return null for non-boolean values', () {
241+
final map = {
242+
'online': 'true',
243+
'simulator': 'false',
244+
};
245+
final sentryDevice = SentryDevice.fromJson(map);
246+
expect(sentryDevice.online, isNull);
247+
expect(sentryDevice.simulator, isNull);
248+
});
249+
250+
test('bool fields accept numeric 0 and 1 as false and true', () {
251+
final map = {
252+
'charging': 1,
253+
'low_memory': 0,
254+
'online': 1.0,
255+
'simulator': 0.0,
256+
};
257+
final sentryDevice = SentryDevice.fromJson(map);
258+
expect(sentryDevice.charging, true);
259+
expect(sentryDevice.lowMemory, false);
260+
expect(sentryDevice.online, true);
261+
expect(sentryDevice.simulator, false);
262+
});
263+
264+
test('bool fields return null for other numeric values', () {
265+
final map = {
266+
'charging': 2,
267+
'low_memory': -1,
268+
'online': 0.5,
269+
};
270+
final sentryDevice = SentryDevice.fromJson(map);
271+
expect(sentryDevice.charging, isNull);
272+
expect(sentryDevice.lowMemory, isNull);
273+
expect(sentryDevice.online, isNull);
274+
});
275+
276+
test('mixed valid and invalid data deserializes partially', () {
277+
final map = {
278+
'name': 'valid name',
279+
'family': 123, // invalid
280+
'battery_level': 75.5,
281+
'orientation': 'invalid', // invalid enum
282+
'online': true,
283+
'charging': 'not a bool', // invalid
284+
'screen_height_pixels': 1920,
285+
'screen_width_pixels': 'not a number', // invalid
286+
'boot_time': 'not a date', // invalid
287+
};
288+
final sentryDevice = SentryDevice.fromJson(map);
289+
290+
// Valid fields should deserialize correctly
291+
expect(sentryDevice.name, 'valid name');
292+
expect(sentryDevice.batteryLevel, 75.5);
293+
expect(sentryDevice.online, true);
294+
expect(sentryDevice.screenHeightPixels, 1920);
295+
296+
// Invalid fields should be null
297+
expect(sentryDevice.family, isNull);
298+
expect(sentryDevice.orientation, isNull);
299+
expect(sentryDevice.charging, isNull);
300+
expect(sentryDevice.screenWidthPixels, isNull);
301+
expect(sentryDevice.bootTime, isNull);
302+
});
155303
});
156304

157305
test('copyWith keeps unchanged', () {

0 commit comments

Comments
 (0)