STRLCPY/0days-in-the-wild

CVE-2021-30858 described the wrong bug, updated the page to clarify
```
Change-Id: I418e43fe6ea580860e4cfcc0f18fda4d31a40ff6
```
Maddie Stone committed 3 years ago

65fcdf04

1 parent 48d34365

Total 1 files

■ ■ ■ ■ ■ ■

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

1		-	# CVE-2021-30858: Use-after-free in WebKit
2		-	Maddie Stone, Google Project Zero
	1	+	# :grimacing: Whoopsie daisy!! :grimacing:
3	2
4		-	## The Basics
	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.
5	4
6		-	Disclosure or Patch Date: 13 September 2021
7	5
8		-	Product: Apple WebKit
	6	+	## ~~CVE-2021-30858: Use-after-free in WebKit~~
	7	+	~~Maddie Stone, Google Project Zero~~
9	8
10		-	Advisory: https://support.apple.com/en-us/HT212808
	9	+	## ~~The Basics~~
11	10
12		-	Affected Versions: pre-Safari 14.1.2, pre-iOS 14.8
	11	+	~~Disclosure or Patch Date: 13 September 2021~~
13	12
14		-	First Patched Version: Safari 14.1.2, iOS 14.8
	13	+	~~Product: Apple WebKit~~
15	14
16		-	Issue/Bug Report: https://bugs.webkit.org/show_bug.cgi?id=229535
	15	+	~~Advisory: https://support.apple.com/en-us/HT212808~~
17	16
18		-	Patch CL: https://github.com/WebKit/WebKit/commit/fbf37d27e313d8d0a150a74cc8fab956eb7f3c59
	17	+	~~Affected Versions: pre-Safari 14.1.2, pre-iOS 14.8~~
19	18
20		-	Bug-Introducing CL: https://github.com/WebKit/WebKit/commit/d5dbfd02054e9f904b27224a598ca1bb8ded5f87
	19	+	~~First Patched Version: Safari 14.1.2, iOS 14.8~~
21	20
22		-	Reporter(s): Anonymous
	21	+	~~Issue/Bug Report: https://bugs.webkit.org/show_bug.cgi?id=229535~~
23	22
24		-	## The Code
	23	+	~~Patch CL: https://github.com/WebKit/WebKit/commit/fbf37d27e313d8d0a150a74cc8fab956eb7f3c59~~
25	24
26		-	Proof-of-concept:
	25	+	~~Bug-Introducing CL: https://github.com/WebKit/WebKit/commit/d5dbfd02054e9f904b27224a598ca1bb8ded5f87~~
27	26
28		-	```javascript
29		-	var fontFace1 = new FontFace("font1", "", {});
30		-	var fontFaceSet = new FontFaceSet([fontFace1]);
31		-	fontFace1.family = "font2";
32		-	```
	27	+	~~Reporter(s): Anonymous~~
33	28
34		-	Exploit sample: N/A
	29	+	## ~~The Code~~
35	30
36		-	Did you have access to the exploit sample when doing the analysis? No
	31	+	~~Proof-of-concept:~~
37	32
38		-	## The Vulnerability
	33	+	~~var fontFace1 = new FontFace("font1", "", {});
	34	+	var fontFaceSet = new FontFaceSet([fontFace1]);
	35	+	fontFace1.family = "font2";~~
39	36
40		-	Bug class: Use-after-free
	37	+	~~Exploit sample: N/A~~
41	38
42		-	Vulnerability details:
43		-	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.
	39	+	~~Did you have access to the exploit sample when doing the analysis? No~~
44	40
45		-	https://github.com/WebKit/WebKit/blob/74bd0da94fa1d31a115bc4ee0e3927d8b2ea571e/Source/WebCore/css/CSSFontFaceSet.cpp#L223
46		-	```c++
47		-	void CSSFontFaceSet::removeFromFacesLookupTable(const CSSFontFace& face, const CSSValueList& familiesToSearchFor)
48		-	{
49		-	for (auto& item : familiesToSearchFor) {
50		-	String familyName = CSSFontFaceSet::familyNameFromPrimitive(downcast<CSSPrimitiveValue>(item.get()));
51		-	if (familyName.isEmpty())
52		-	continue;
	41	+	## ~~The Vulnerability~~
53	42
54		-	auto iterator = m_facesLookupTable.find(familyName);
55		-	ASSERT(iterator != m_facesLookupTable.end());
56		-	bool found = false;
57		-	for (size_t i = 0; i < iterator->value.size(); ++i) {
58		-	if (iterator->value[i].ptr() == &face) {
59		-	found = true;
60		-	iterator->value.remove(i);
61		-	break;
62		-	}
63		-	}
64		-	ASSERT_UNUSED(found, found);
65		-	if (!iterator->value.size())
66		-	m_facesLookupTable.remove(iterator);
67		-	}
68		-	}
	43	+	~~Bug class: Use-after-free~~
69	44
70		-	```
	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.~~
71	47
72		-	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.
	48	+	~~https://github.com/WebKit/WebKit/blob/74bd0da94fa1d31a115bc4ee0e3927d8b2ea571e/Source/WebCore/css/CSSFontFaceSet.cpp#L223~~
73	49
74		-	Patch analysis:
	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.~~
75	51
76		-	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.
	52	+	~~Patch analysis:~~
77	53
78		-	Thoughts on how this vuln might have been found _(fuzzing, code auditing, variant analysis, etc.)_:
	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.~~
79	55
80		-	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.
	56	+	~~Thoughts on how this vuln might have been found _(fuzzing, code auditing, variant analysis, etc.)_:~~
81	57
82		-	(Historical/present/future) context of bug:
	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.~~
83	59
84		-	## The Exploit
	60	+	~~(Historical/present/future) context of bug:~~
85	61
86		-	(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).)
	62	+	## ~~The Exploit~~
87	63
88		-	Exploit strategy (or strategies):
	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).)~~
89	65
90		-	N/A no access to exploit sample
	66	+	~~Exploit strategy (or strategies):~~
91	67
92		-	Exploit flow:
	68	+	~~N/A no access to exploit sample~~
93	69
94		-	Known cases of the same exploit flow:
	70	+	~~Exploit flow:~~
95	71
96		-	Part of an exploit chain?
	72	+	~~Known cases of the same exploit flow:~~
97	73
98		-	## The Next Steps
	74	+	~~Part of an exploit chain?~~
99	75
100		-	### Variant analysis
	76	+	## ~~The Next Steps~~
101	77
102		-	Areas/approach for variant analysis (and why):
	78	+	### ~~Variant analysis~~
103	79
104		-	* 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.
105		-	* Fuzz the `FontFace` components.
	80	+	~~Areas/approach for variant analysis (and why):~~
106	81
107		-	Found variants: N/A
	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.~~
108	84
109		-	### Structural improvements
	85	+	~~Found variants: N/A~~
110	86
111		-	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.?
	87	+	### ~~Structural improvements~~
112	88
113		-	Ideas to kill the bug class:
	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.?~~
114	90
115		-	* 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.
116		-	* Memory-safe languages. This is a memory corruption bug so switching to a memory safe language would also prevent this type of vulnerability.
	91	+	~~Ideas to kill the bug class:~~
117	92
118		-	Ideas to mitigate the exploit flow:
	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.~~
119	95
120		-	N/A
	96	+	~~Ideas to mitigate the exploit flow: N/A~~
121	97
122		-	Other potential improvements:
	98	+	~~Other potential improvements:~~
123	99
124		-	### 0-day detection methods
	100	+	### ~~0-day detection methods~~
125	101
126		-	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.
	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.~~
127	103
128		-	## Other References
	104	+	## ~~Other References~~
129	105

CVE-2021-30858 described the wrong bug, updated the page to clarify