From 90888765254c1fa54fa39e2eb383aa3436a2e8f8 Mon Sep 17 00:00:00 2001 From: Lorenz Bauer Date: Thu, 21 Dec 2017 11:24:07 +0100 Subject: [PATCH] Don't use untrusted lengths from Header to pre-allocate (#610) We currently use information from a potential attacker to pre-allocate slices for the Question, Answer, etc. sections. This allows an attacker to force allocation of several MiB per parsed Msg. Instead, don't pre-allocate those slices. append() always allocates in powers of two, which is probably the best we can do. Fixes #609. --- msg.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/msg.go b/msg.go index 9419efd9..975dde78 100644 --- a/msg.go +++ b/msg.go @@ -612,8 +612,8 @@ func UnpackRR(msg []byte, off int) (rr RR, off1 int, err error) { // If we cannot unpack the whole array, then it will return nil func unpackRRslice(l int, msg []byte, off int) (dst1 []RR, off1 int, err error) { var r RR - // Optimistically make dst be the length that was sent - dst := make([]RR, 0, l) + // Don't pre-allocate, l may be under attacker control + var dst []RR for i := 0; i < l; i++ { off1 := off r, off, err = UnpackRR(msg, off) @@ -820,9 +820,10 @@ func (dns *Msg) Unpack(msg []byte) (err error) { return nil } - // Optimistically use the count given to us in the header - dns.Question = make([]Question, 0, int(dh.Qdcount)) - + // Qdcount, Ancount, Nscount, Arcount can't be trusted, as they are + // attacker controlled. This means we can't use them to pre-allocate + // slices. + dns.Question = nil for i := 0; i < int(dh.Qdcount); i++ { off1 := off var q Question