Commit 2fb46b16 authored by Lorenz Bauer's avatar Lorenz Bauer Committed by Brad Fitzpatrick

dns/dnsmessage: don't use untrusted data to pre-allocate slices

We mustn't use data from p.header to pre-allocate slices for Message.Question, etc.
Otherwise an attacker can force the allocation of several MiB per parsed message,
which can lead to a DoS via putting pressure on the GC.

Fixes golang/go#23214

Change-Id: I6c99577f625b08331b438533adb6b8167bcd1ec5
Reviewed-on: https://go-review.googlesource.com/85135Reviewed-by: 's avatarIan Gudger <igudger@google.com>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
parent b417086c
...@@ -436,7 +436,13 @@ func (p *Parser) Question() (Question, error) { ...@@ -436,7 +436,13 @@ func (p *Parser) Question() (Question, error) {
// AllQuestions parses all Questions. // AllQuestions parses all Questions.
func (p *Parser) AllQuestions() ([]Question, error) { func (p *Parser) AllQuestions() ([]Question, error) {
qs := make([]Question, 0, p.header.questions) // Multiple questions are valid according to the spec,
// but servers don't actually support them. There will
// be at most one question here.
//
// Do not pre-allocate based on info in p.header, since
// the data is untrusted.
qs := []Question{}
for { for {
q, err := p.Question() q, err := p.Question()
if err == ErrSectionDone { if err == ErrSectionDone {
...@@ -492,7 +498,16 @@ func (p *Parser) Answer() (Resource, error) { ...@@ -492,7 +498,16 @@ func (p *Parser) Answer() (Resource, error) {
// AllAnswers parses all Answer Resources. // AllAnswers parses all Answer Resources.
func (p *Parser) AllAnswers() ([]Resource, error) { func (p *Parser) AllAnswers() ([]Resource, error) {
as := make([]Resource, 0, p.header.answers) // The most common query is for A/AAAA, which usually returns
// a handful of IPs.
//
// Pre-allocate up to a certain limit, since p.header is
// untrusted data.
n := int(p.header.answers)
if n > 20 {
n = 20
}
as := make([]Resource, 0, n)
for { for {
a, err := p.Answer() a, err := p.Answer()
if err == ErrSectionDone { if err == ErrSectionDone {
...@@ -533,7 +548,16 @@ func (p *Parser) Authority() (Resource, error) { ...@@ -533,7 +548,16 @@ func (p *Parser) Authority() (Resource, error) {
// AllAuthorities parses all Authority Resources. // AllAuthorities parses all Authority Resources.
func (p *Parser) AllAuthorities() ([]Resource, error) { func (p *Parser) AllAuthorities() ([]Resource, error) {
as := make([]Resource, 0, p.header.authorities) // Authorities contains SOA in case of NXDOMAIN and friends,
// otherwise it is empty.
//
// Pre-allocate up to a certain limit, since p.header is
// untrusted data.
n := int(p.header.authorities)
if n > 10 {
n = 10
}
as := make([]Resource, 0, n)
for { for {
a, err := p.Authority() a, err := p.Authority()
if err == ErrSectionDone { if err == ErrSectionDone {
...@@ -574,7 +598,16 @@ func (p *Parser) Additional() (Resource, error) { ...@@ -574,7 +598,16 @@ func (p *Parser) Additional() (Resource, error) {
// AllAdditionals parses all Additional Resources. // AllAdditionals parses all Additional Resources.
func (p *Parser) AllAdditionals() ([]Resource, error) { func (p *Parser) AllAdditionals() ([]Resource, error) {
as := make([]Resource, 0, p.header.additionals) // Additionals usually contain OPT, and sometimes A/AAAA
// glue records.
//
// Pre-allocate up to a certain limit, since p.header is
// untrusted data.
n := int(p.header.additionals)
if n > 10 {
n = 10
}
as := make([]Resource, 0, n)
for { for {
a, err := p.Additional() a, err := p.Additional()
if err == ErrSectionDone { if err == ErrSectionDone {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment