[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointi
From: |
Samuel Thibault |
Subject: |
Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload |
Date: |
Thu, 22 Aug 2019 20:33:13 +0200 |
User-agent: |
NeoMutt/20170609 (1.8.3) |
Hello,
Philippe Mathieu-Daudé, le jeu. 22 août 2019 16:41:33 +0200, a ecrit:
> Later the newly calculated pointer q is converted into ip structure
> and values are modified, Due to the wrong calculation of the delta,
> ip will be pointing to incorrect location and ip_src and ip_dst can
> be used to write controlled data onto the calculated location. This
> may also crash qemu if the calculated ip is located in unmaped area.
That does not seem to be related to this:
> Do not queue fragments pointing out of the original payload to avoid
> to calculate the variable delta.
I don't understand the relation with having to calculate delta.
> diff --git a/src/ip_input.c b/src/ip_input.c
> index 7364ce0..ee52085 100644
> --- a/src/ip_input.c
> +++ b/src/ip_input.c
> @@ -304,6 +304,19 @@ static struct ip *ip_reass(Slirp *slirp, struct ip *ip,
> struct ipq *fp)
> ip_deq(q->ipf_prev);
> }
>
> + /*
> + * If we received the first fragment, we know the original
> + * payload size.
? We only know the total payload size when receiving the last fragment
(payload = offset*8 + size).
> Verify fragments are within our payload.
By construction of the protocol, fragments can only be within the
payload, since it's the last fragment which provides the payload size.
> + for (q = fp->frag_link.next; q != (struct ipasfrag*)&fp->frag_link;
> + q = q->ipf_next) {
> + if (!q->ipf_off && q->ipf_len) {
> + if (ip->ip_off + ip->ip_len >= q->ipf_len) {
> + goto dropfrag;
> + }
> + }
> + }
Fragments are kept in order, there is no need to go around the list to
find the fragment with offset zero, if it is there it is the first one.
Did you make your test with commit 126c04acbabd ("Fix heap overflow in
ip_reass on big packet input") applied?
Samuel
- [Qemu-stable] [PATCH 0/2] slirp: Fix heap buffer overflow during packet reassembly (CVE-2019-14378), Philippe Mathieu-Daudé, 2019/08/22
- [Qemu-stable] [RFC PATCH 2/2] Delay crash when mbufs are corrupted, Philippe Mathieu-Daudé, 2019/08/22
- [Qemu-stable] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, Philippe Mathieu-Daudé, 2019/08/22
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload,
Samuel Thibault <=
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, Philippe Mathieu-Daudé, 2019/08/23
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, Samuel Thibault, 2019/08/25
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, P J P, 2019/08/29
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, Philippe Mathieu-Daudé, 2019/08/29
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, Philippe Mathieu-Daudé, 2019/08/29
- Re: [Qemu-stable] [Slirp] [PATCH 1/2] Do not reassemble fragments pointing outside of the original payload, Philippe Mathieu-Daudé, 2019/08/29