Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

net/mail: mail ParseAddressList can bypass and panic #43714

Closed
cor0ps opened this issue Jan 15, 2021 · 5 comments
Closed

net/mail: mail ParseAddressList can bypass and panic #43714

cor0ps opened this issue Jan 15, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@cor0ps
Copy link

cor0ps commented Jan 15, 2021 •
edited by ALTree

What version of Go are you using (go version)?

$ go version
go version go1.15.4 windows/amd64

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
C:\Users\xx\go\src\awesomeProject1>go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\xx\AppData\Local\go-build
set GOENV=C:\Users\xx\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\xx\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\xx\go
set GOPRIVATE=
set GOPROXY=direct
set GOROOT=C:\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\xx\go\src\awesomeProject1\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\xx\AppData\Local\Temp\go-build958360038=/tmp/go-build -gno-record-gcc-switches

What did you do?

https://play.golang.org/p/efQXFJo-TIC

package main

import (
	"net/mail"
	"strings"
)

func main() {
	//"":;
	data := "\"\":;"
	list, err := mail.ParseAddressList(data)
	if err != nil {
		return
	}
	var addrs []string
	for _, addr := range list {
		addrs = append(addrs, addr.String())
	}
	_, err = mail.ParseAddressList(strings.Join(addrs, ","))
	if err != nil {
		panic(err)
	}

}

What did you expect to see?

first input "":; can bypass

if handleGroup {
   if p.consume(':') {
      return p.consumeGroupList()
   }
}

second is panic: mail: no address

What did you see instead?

panic: mail: no address
@cor0ps cor0ps changed the title mail ParseAddressList can bypass mail ParseAddressList can bypass and panic Jan 15, 2021
@ALTree ALTree changed the title mail ParseAddressList can bypass and panic net/mail: mail ParseAddressList can bypass and panic Jan 15, 2021
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 15, 2021
@mengzhuo
Copy link
Contributor

It seems we should check on addrs number at the end of parseAddressList

https://golang.org/src/net/mail/message.go?s=5170:5224#L307

		if p.empty() {
			break
		}
	}
	return list, nil // we shoud check it here

I can send a CL for this issue , Is it OK for you ? @cor0ps

@odeke-em
Copy link
Member

Thank you for this report @cor0ps, thanks @mengzhuo and @ALTree for the triage! @cuonglm perhaps we should also place this on our list for fuzzing?

@cor0ps
Copy link
Author

cor0ps commented Jan 16, 2021

@mengzhuo mengzhuo you can make a issuse and have questions can contact with me wechat：SuperTao99

	if p.empty() {
			break
		}
	}
	return list, nil // we shoud check it here

I aggree with you

@cor0ps
Copy link
Author

cor0ps commented Jan 16, 2021 •
edited

@odeke-em I found this use gofuzz tools and i think this have a securtiy problem and this can get a CVE id ？ thanks

@mengzhuo
Copy link
Contributor

@odeke-em I found this use gofuzz tools and i think this have a securtiy problem and this can get a CVE id ？ thanks

I don't think it's a CVE, RFC 5322 stated that empty group is allowed.

https://tools.ietf.org/html/rfc5322#section-3.4

@cor0ps cor0ps closed this as completed Feb 22, 2021
@golang golang locked and limited conversation to collaborators Feb 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mengzhuo @ALTree @odeke-em @gopherbot @cor0ps