Re: [Xen-devel] [PATCH 4 00/16] XSA55 libelf fixes for unstable

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Matthew Daley writes ("Re: [PATCH 4 00/16] XSA55 libelf fixes for unstable"):
> Looks like there's another issue that needs fixing up in this XSA
> (surprise!):

Urgh.

> setup_hypercall_page (in xc_dom_boot.c) calls xc_dom_p2m_guest with an
> unchecked, user-controlled pfn:
...
> Here, the silly dom->parms.virt_base is leading to an out-of-bounds
> array access to the guest p2m table.

Thanks.  I have a proposed fix for this, below.  I haven't tested it.
Can you do so easily ?  It seems a bit remote from the problem but I
think it should suffice.

I searched the code for (a) other callers of xc_dom_p2m_{guest,host}
and (b) other uses of the parms field from the ELF and I think the
rest are probably OK, although I didn't spend a great deal of time on
it.

>  (Also, perhaps
> dom->parms.virt_hypercall should be being compared to UNSET_ADDR, not
> -1, on line 47.)

Ian.

From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Subject: [PATCH] libxc: range checks in xc_dom_p2m_host and _guest

These functions take guest pfns and look them up in the p2m.  They did
no range checking.

However, some callers, notably xc_dom_boot.c:setup_hypercall_page want
to pass untrusted guest-supplied value(s).  It is most convenient to
detect this here and return INVALID_MFN.

This is part of the fix to a security issue, XSA-55.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

---
 tools/libxc/xc_dom.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 567913f..239ef33 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -341,6 +341,8 @@ static inline xen_pfn_t xc_dom_p2m_host(struct xc_dom_image 
*dom, xen_pfn_t pfn)
 {
     if (dom->shadow_enabled)
         return pfn;
+    if (pfn >= dom->rambase_pfn + dom->total_pages)
+        return INVALID_MFN;
     return dom->p2m_host[pfn - dom->rambase_pfn];
 }
 
@@ -349,6 +351,8 @@ static inline xen_pfn_t xc_dom_p2m_guest(struct 
xc_dom_image *dom,
 {
     if (xc_dom_feature_translated(dom))
         return pfn;
+    if (pfn >= dom->rambase_pfn + dom->total_pages)
+        return INVALID_MFN;
     return dom->p2m_host[pfn - dom->rambase_pfn];
 }
 
-- 
tg: (28e5cf0..) xsa55/xc_dom_p2m-check (depends on: xsa55/xc-malloc-check)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel