STRLCPY/0days-in-the-wild

Re-writing CVE-2021-30858 RCA

The originally posted CVE-2021-30858 RCA was actually analyzing
CVE-2021-30889 fixed in iOS 15.1. Whoopsie Daisy. Thanks to Apple for
calling attention to the issue and pointing out the correct patch for
CVE-2021-30858.

Change-Id: I5cc7f4d9d206fd5bf6f9c708b067c6caba9bd3d9

Maddie Stone committed 2 years ago

1a5fdbff

1 parent a59d3912

Total 1 files

■ ■ ■ ■ ■ ■

0day-RCAs/2021/CVE-2021-30858.md

1		-	# :grimacing: Whoopsie daisy!! :grimacing:
	1	+	## CVE-2021-30858: WebKit use-after-free in IndexedDB
	2	+	Maddie Stone, Google Project Zero
	3	+
	4	+	## The Basics
	5	+
	6	+	Disclosure or Patch Date: 13 September 2021
	7	+
	8	+	Product: Apple WebKit
	9	+
	10	+	Advisory: https://support.apple.com/en-us/HT212808
	11	+
	12	+	Affected Versions: pre-Safari 14.1.2, pre-iOS 14.8
2	13
3		-	## Turns out this RCA was describing a different vulnerability, not CVE-2021-30858. Thanks to Apple for letting me know! RCA for the correct bug is forthcoming.
	14	+	First Patched Version: Safari 14.1.2, iOS 14.8
4	15
	16	+	Issue/Bug Report: https://bugs.webkit.org/show_bug.cgi?id=229375
5	17
6		-	## ~~CVE-2021-30858: Use-after-free in WebKit~~
7		-	~~Maddie Stone, Google Project Zero~~
	18	+	Patch CL: https://trac.webkit.org/changeset/281384/webkit
	19	+
	20	+	Bug-Introducing CL: ??
8	21
9		-	## ~~The Basics~~
	22	+	Reporter(s): Anonymous
10	23
11		-	~~Disclosure or Patch Date: 13 September 2021~~
	24	+	## The Code
12	25
13		-	~~Product: Apple WebKit~~
	26	+	Proof-of-concept:
14	27
15		-	~~Advisory: https://support.apple.com/en-us/HT212808~~
	28	+	index.html
	29	+	```html
	30	+	<html>
	31	+	<script>
	32	+	w = new Worker('idbworker.js');
	33	+	</script>
	34	+	</html>
	35	+	```
16	36
17		-	~~Affected Versions: pre-Safari 14.1.2, pre-iOS 14.8~~
	37	+	idbworker.js
	38	+	```javascript
	39	+	function freememory() {
	40	+	for (var i = 0; i < 1000; i++) {
	41	+	a = new Uint8Array(1024*1024);
	42	+	}
	43	+	}
18	44
19		-	~~First Patched Version: Safari 14.1.2, iOS 14.8~~
	45	+	let ev = new Event('mine');
	46	+	let req = indexedDB.open('db');
	47	+	req.dispatchEvent(ev);
	48	+	req = 0;
	49	+	ev = 0;
	50	+	freememory();
	51	+	```
20	52
21		-	~~Issue/Bug Report: https://bugs.webkit.org/show_bug.cgi?id=229535~~
	53	+	Exploit sample: N/A
22	54
23		-	~~Patch CL: https://github.com/WebKit/WebKit/commit/fbf37d27e313d8d0a150a74cc8fab956eb7f3c59~~
	55	+	Did you have access to the exploit sample when doing the analysis? No
24	56
25		-	~~Bug-Introducing CL: https://github.com/WebKit/WebKit/commit/d5dbfd02054e9f904b27224a598ca1bb8ded5f87~~
	57	+	## The Vulnerability
26	58
27		-	~~Reporter(s): Anonymous~~
	59	+	Bug class: Use-after-free
28	60
29		-	## ~~The Code~~
	61	+	Vulnerability details:
	62	+	There is a use-after-free of the `IDBOpenDBRequest` due to a cross-thread task using a raw reference. `IDBRequest` is the base class of `IDBOpenDBRequest`. The state of the `IDBRequest` is able to be changed in `dispatchEvent` by a script-generated custom event, which leads to the `IDBRequest` being freed too early and thus the use-after-free.
30	63
31		-	~~Proof-of-concept:~~
	64	+	Prior to this vulnerability being fixed, there were two template options for the `createCrossThreadTask` function:
	65	+	1. The callee object is a derived class of `ThreadSafeRefCounted<T>` so the cross-thread task will use a `RefPtr` for the callee ([source](https://github.com/WebKit/WebKit/blame/f38367dfe2a83b4cda78235ffd1dc3001743c36e/Source/WTF/wtf/CrossThreadTask.h#L90)):
32	66
33		-	~~var fontFace1 = new FontFace("font1", "", {});
34		-	var fontFaceSet = new FontFaceSet([fontFace1]);
35		-	fontFace1.family = "font2";~~
	67	+	```c++
	68	+	template<typename T, typename std::enable_if<std::is_base_of<ThreadSafeRefCounted<T>, T>::value, int>::type = 0, typename... Parameters, typename... Arguments>
	69	+	CrossThreadTask createCrossThreadTask(T& callee, void (T::*method)(Parameters...), const Arguments&... arguments)
	70	+	{
	71	+	return CrossThreadTask([callee = makeRefPtr(&callee), method, arguments = std::make_tuple(crossThreadCopy(arguments)...)]() mutable {
	72	+	callMemberFunctionForCrossThreadTask(callee.get(), method, WTFMove(arguments));
	73	+	});
	74	+	}
	75	+	```
36	76
37		-	~~Exploit sample: N/A~~
	77	+	2. The callee object is NOT a derived class of `ThreadSafeRefCounted<T>` so the cross-thread task will use a raw reference for the callee ([source](https://github.com/WebKit/WebKit/blame/f38367dfe2a83b4cda78235ffd1dc3001743c36e/Source/WTF/wtf/CrossThreadTask.h#L98)):
38	78
39		-	~~Did you have access to the exploit sample when doing the analysis? No~~
	79	+	```c++
	80	+	template<typename T, typename std::enable_if<!std::is_base_of<ThreadSafeRefCounted<T>, T>::value, int>::type = 0, typename... Parameters, typename... Arguments>
	81	+	CrossThreadTask createCrossThreadTask(T& callee, void (T::*method)(Parameters...), const Arguments&... arguments)
	82	+	{
	83	+	return CrossThreadTask([callee = &callee, method, arguments = std::make_tuple(crossThreadCopy(arguments)...)]() mutable {
	84	+	callMemberFunctionForCrossThreadTask(callee, method, WTFMove(arguments));
	85	+	});
	86	+	}
	87	+	```
40	88
41		-	## ~~The Vulnerability~~
	89	+	To trigger this vulnerability, we're using a callee object of `IDBOpenDBRequest`. `IDBOpenDBRequest` is a derived class from `IDBRequest` ([IDBOpenDBRequest.h#L35](https://github.com/WebKit/WebKit/blame/f38367dfe2a83b4cda78235ffd1dc3001743c36e/Source/WebCore/Modules/indexeddb/IDBOpenDBRequest.h#L35)). `IDBRequest` is a derived class from `ThreadSafeRefCounted<IDBRequest>` ([IDBRequest.h#L63](https://github.com/WebKit/WebKit/blame/f38367dfe2a83b4cda78235ffd1dc3001743c36e/Source/WebCore/Modules/indexeddb/IDBRequest.h#L63)).
42	90
43		-	~~Bug class: Use-after-free~~
	91	+	While the intention was that an `IDBOpenDBRequest` callee would use template #1, the `RefPtr`, #2 was actually used because `IDBOpenDBRequest` is not derived from `ThreadSafeRefCounted<IDBOpenDBRequest>`, it's actually derived from `ThreadSafeRefCounted<IDBRequest>`. Therefore #2 with the raw reference was used.
44	92
45		-	~~Vulnerability details:~~
46		-	~~The vulnerability is a use-after-free due to an unchecked `end()` iterator. There was an assert statement: `ASSERT(iterator != m_facesLookupTable.end());`, but `ASSERT`s don't do anything in release builds. Therefore, even if `iterator == m_facesLookupTable.end()` in the release build, nothing would happen and `iterator` would still be used.~~
	93	+	Patch analysis:
47	94
48		-	~~https://github.com/WebKit/WebKit/blob/74bd0da94fa1d31a115bc4ee0e3927d8b2ea571e/Source/WebCore/css/CSSFontFaceSet.cpp#L223~~
	95	+	There are two parts to the patch:
	96	+	1. In `IDBRequest::dispatchEvent` a check is added to only change the state of the request if the event is trusted, i.e. internally generated.
	97	+	2. In `CrossThreadTask::createCrossThreadTask` the template is modified to ensure that the callee object, in this case the `IDBOpenDBRequest`, is a `RefPtr` and therefore protected, rather than being a raw reference. This is done by changing the templates to use `ThreadSafeRefCountedBase` instead of `ThreadSafeRefCounted<T>`.
49	98
50		-	~~In `FontFaceSet` a `FontFace` is not added to the faces lookup table in `addToFacesLookupTable` if the font has already been deemed to be invalid. However, `removeFromFacesLookupTable` would still attempt to remove the font, leading to the use-after-free.~~
	99	+	Thoughts on how this vuln might have been found _(fuzzing, code auditing, variant analysis, etc.)_:
51	100
52		-	~~Patch analysis:~~
	101	+	This bug was most likely found via fuzzing. The trigger for this vulnerability uses all common patterns that would be known to most fuzzers. It is also possible though that the attackers found the vulnerability after seeing the similar Chrome & WebKit bugs.
53	102
54		-	~~The patch changes the `ASSERT` to an if clause. The function will return if `iterator == m_facesLookupTable.end()`, since the item it wishes to remove is not found in the table.~~
	103	+	(Historical/present/future) context of bug:
55	104
56		-	~~Thoughts on how this vuln might have been found _(fuzzing, code auditing, variant analysis, etc.)_:~~
	105	+	* In December 2019, what seems to be a similar bug was found in Chrome by ClusterFuzz: https://bugs.chromium.org/p/chromium/issues/detail?id=1032890
	106	+	* In March 2019, what seems to be a similar [bug](https://bugs.webkit.org/show_bug.cgi?id=195563) was found in WebKit. In this case, the use-after-free was on the `IDBDatabase` object. The [patch](https://github.com/WebKit/WebKit/commit/9dc1355f53270b710accba13b54300cca74136c6) included adding the template to `CrossThreadTask::createCrossThreadTask` for callees that are a derived class of `ThreadSafeRefCounted<T>`.
57	107
58		-	~~It seems equally likely that this vulnerability could have been found via fuzzing or code auditing. The trigger is only 3 lines long so it seems like that a fuzzer could have triggered the vulnerability.~~
	108	+	## The Exploit
59	109
60		-	~~(Historical/present/future) context of bug:~~
	110	+	(The terms exploit primitive, exploit strategy, exploit technique, and exploit flow are [defined here](https://googleprojectzero.blogspot.com/2020/06/a-survey-of-recent-ios-kernel-exploits.html).)
61	111
62		-	## ~~The Exploit~~
	112	+	Exploit strategy (or strategies):
63	113
64		-	~~(The terms exploit primitive, exploit strategy, exploit technique, and exploit flow are [defined here](https://googleprojectzero.blogspot.com/2020/06/a-survey-of-recent-ios-kernel-exploits.html).)~~
	114	+	N/A no access to exploit sample
65	115
66		-	~~Exploit strategy (or strategies):~~
	116	+	Exploit flow:
67	117
68		-	~~N/A no access to exploit sample~~
	118	+	Known cases of the same exploit flow:
69	119
70		-	~~Exploit flow:~~
	120	+	Part of an exploit chain?
71	121
72		-	~~Known cases of the same exploit flow:~~
	122	+	## The Next Steps
73	123
74		-	~~Part of an exploit chain?~~
	124	+	### Variant analysis
75	125
76		-	## ~~The Next Steps~~
	126	+	Areas/approach for variant analysis (and why):
	127	+	1. Check whether other `dispatchEvent` functions change state based on non-trusted (custom/script-generated) events.
	128	+	2. Check whether tasks that take place in threads other than the origin thread use raw references or pointers.
	129	+	3. General auditing of IndexedDB code.
77	130
78		-	### ~~Variant analysis~~
	131	+	Found variants: N/A
79	132
80		-	~~Areas/approach for variant analysis (and why):~~
	133	+	### Structural improvements
81	134
82		-	* ~~Look for other places where there is an `ASSERT` checking that the result of `find() != end()`, but the result can still be used even if that `ASSERT` fails.~~
83		-	* ~~Fuzz the `FontFace` components.~~
	135	+	What are structural improvements such as ways to kill the bug class, prevent the introduction of this vulnerability, mitigate the exploit flow, make this type of vulnerability harder to exploit, etc.?
84	136
85		-	~~Found variants: N/A~~
	137	+	Ideas to kill the bug class:
86	138
87		-	### ~~Structural improvements~~
	139	+	* Memory Tagging could potentially prevent this use-after-free.
88	140
89		-	~~What are structural improvements such as ways to kill the bug class, prevent the introduction of this vulnerability, mitigate the exploit flow, make this type of vulnerability harder to exploit, etc.?~~
	141	+	Ideas to mitigate the exploit flow: N/A
90	142
91		-	~~Ideas to kill the bug class:~~
	143	+	Other potential improvements:
92	144
93		-	* ~~Change any `ASSERT` statements that verifies that a value doesn't equal `end()` to `RELEASE_ASSERT` statements. These situations should be caught prior to the `RELEASE_ASSERT`, but at least if it happens to not be, the `RELEASE_ASSERT` would prevent exploitation.~~
94		-	* ~~Memory-safe languages. This is a memory corruption bug so switching to a memory safe language would also prevent this type of vulnerability.~~
	145	+	* Monitoring fixes in other platforms who have similar implementations of code. For example, in [January 2020 Chrome](https://bugs.chromium.org/p/chromium/issues/detail?id=1032890) fixed that [`IDBRequest::DispatchEventInternal` changed state in IndexedDB objects based on untrusted events](https://chromium.googlesource.com/chromium/src.git/+/96f63f777dfc517e46cd41edd9f8205f42055aba%5E%21/#F2). Making that change in WebKit would have fixed this vulnerability.
95	146
96		-	~~Ideas to mitigate the exploit flow: N/A~~
	147	+	### 0-day detection methods
97	148
98		-	~~Other potential improvements:~~
	149	+	These may have high rates of false positives, but here are some ideas for detecting:
99	150
100		-	### ~~0-day detection methods~~
	151	+	* Looking for scripts that have functions specifically to trigger garbage collection.
	152	+	* Dispatching custom events to IndexedDB objects.
101	153
102		-	~~There doesn't seem to be any really great options for detection due to the trigger for the vulnerability not doing anything that out of the ordinary other than running the samples in a debug WebKit build.~~
	154	+	## Other References
103	155
104		-	## ~~Other References~~
105	156