Re: [Qemu-devel] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload

[Samuel Thibault <samuel.thibault@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

Hello,

Philippe Mathieu-Daudé, le ven. 23 août 2019 17:15:32 +0200, a ecrit:
> > Did you make your test with commit 126c04acbabd ("Fix heap overflow in
> > ip_reass on big packet input") applied?
> 
> Yes, unfortunately it doesn't fix the issue.

Ok.

Could you try the attached patch?  There was a use-after-free.  Without
it, I can indeed crash qemu with the given exploit.  With it I don't
seem to be able to crash it (trying in a loop for several minutes).

Samuel

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 828 bytes --]

diff --git a/src/ip_input.c b/src/ip_input.c
index 7364ce0..aa514ae 100644
--- a/src/ip_input.c
+++ b/src/ip_input.c
@@ -292,6 +292,7 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp)
      */
     while (q != (struct ipasfrag *)&fp->frag_link &&
            ip->ip_off + ip->ip_len > q->ipf_off) {
+        struct ipasfrag *prev;
         i = (ip->ip_off + ip->ip_len) - q->ipf_off;
         if (i < q->ipf_len) {
             q->ipf_len -= i;
@@ -299,9 +300,10 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp)
             m_adj(dtom(slirp, q), i);
             break;
         }
+        prev = q;
         q = q->ipf_next;
-        m_free(dtom(slirp, q->ipf_prev));
-        ip_deq(q->ipf_prev);
+        ip_deq(prev);
+        m_free(dtom(slirp, prev));
     }
 
 insert: