Concurrency and security issue in gomail.v1

[uakfdotb]

gomail.v1 has a concurrency issue when sending multiple e-mails in parallel, where the body of one e-mail gets delivered to the intended recipient of another e-mail. This can leak sensitive information, and thus is also a security issue.

The problem arises from a concurrency issue in gomail.v1 in flattenHeader:

https://github.com/go-gomail/gomail/blob/11b919ab4933936a28fb6aeda5c6523091266f37/mailer.go#L126C1-L150C2

func flattenHeader(msg *mail.Message, bcc string) []byte {
	buf := getBuffer()
	defer putBuffer(buf)
	// ... (write data to buf)
	return buf.Bytes()
}

This function reserves a buffer, writes data to it, and then returns the contents.

However, after returning the contents, the buffer is recycled and can be used by another thread. The underlying bytes are shared between the recycled buffer and the []byte that is returned.

Here is a minimal example showing the problem:

package main

import "bytes"
import "fmt"
import "sync"

var bufPool = sync.Pool{
	New: func() interface{} {
		return new(bytes.Buffer)
	},
}

func getBuffer() *bytes.Buffer {
	return bufPool.Get().(*bytes.Buffer)
}

func putBuffer(buf *bytes.Buffer) {
	if buf.Len() > 1024 {
		return
	}
	buf.Reset()
	bufPool.Put(buf)
}

func getAsBytes(s string) []byte {
	buf := getBuffer()
	defer putBuffer(buf)
	buf.WriteString(s)
	return buf.Bytes()
}

func main() {
	bytes1 := getAsBytes("testing")
	bytes2 := getAsBytes("hello")
	fmt.Println("bytes1", string(bytes1))
	fmt.Println("bytes2", string(bytes2))
}

Here, the getBuffer/putBuffer logic is copied from https://github.com/go-gomail/gomail/blob/v1/gomail.go.

It is expected to print "bytes1 testing" followed by "bytes2 hello", but instead it prints "bytes1 hellong" followed by "bytes2 hello".

A security issue arises where the contents of an e-mail intended for one e-mail address are sent to another e-mail address. This arises because flattenHeader is used to send e-mail:

func (m *Mailer) Send(msg *Message) error {
	message := msg.Export()
	// ...
	recipients, bcc, err := getRecipients(message)
	// ...
	h := flattenHeader(message, "")
	body, err := ioutil.ReadAll(message.Body)
	// ...
	mail := append(h, body...)
	if err := m.send(m.addr, m.auth, from, recipients, mail); err != nil {
		return err
	}
	// ...
}

Here is the timeline of an example security problem:

1. We call Send(...) with an e-mail for user1@example.com. Let's call this Send1.
2. Send1 reads the recipients which is user1@example.com
3. Send1 calls flattenHeader and appends the body to the same byte buffer.
4. Before the e-mail is sent, we call Send(...) again (Send2) for user2@example.com.
5. Send2 calls flattenHeader and then appends the second e-mail's body. The same byte buffer is recycled, overwriting the original contents.
6. Now Send1 makes the m.send call. The recipient is set to user1@example.com (since it has already been read in step 2), but the contents of the e-mail body are that of the second call (since the bytes have been overwritten).

This can easily be fixed (i.e. remove the getBuffer/putBuffer system and just create a new buffer on each call), but since this project and especially v1 is no longer maintained, a better solution is to switch to another library such as https://github.com/wneessen/go-mail