Merge pull request #388 from wneessen/bug/filename-sanitization

Improve filename sanitization in MIME headers

Author: wneessen

msgwriter.go

@@ -273,7 +273,7 @@ func (mw *msgWriter) addFiles(files []*File, isAttachment bool) {
 				mimeType = string(file.ContentType)
 			}
 			file.setHeader(HeaderContentType, fmt.Sprintf(`%s; name="%s"`, mimeType,
-				mw.encoder.Encode(mw.charset.String(), file.Name)))
+				mw.encoder.Encode(mw.charset.String(), sanitizeFilename(file.Name))))
 		}
 
 		if _, ok := file.getHeader(HeaderContentTransferEnc); !ok {
@@ -285,7 +285,7 @@ func (mw *msgWriter) addFiles(files []*File, isAttachment bool) {
 
 		if file.Desc != "" {
 			if _, ok := file.getHeader(HeaderContentDescription); !ok {
-				file.setHeader(HeaderContentDescription, file.Desc)
+				file.setHeader(HeaderContentDescription, mw.encoder.Encode(mw.charset.String(), file.Desc))
 			}
 		}
 
@@ -295,12 +295,12 @@ func (mw *msgWriter) addFiles(files []*File, isAttachment bool) {
 				disposition = "attachment"
 			}
 			file.setHeader(HeaderContentDisposition, fmt.Sprintf(`%s; filename="%s"`,
-				disposition, mw.encoder.Encode(mw.charset.String(), file.Name)))
+				disposition, mw.encoder.Encode(mw.charset.String(), sanitizeFilename(file.Name))))
 		}
 
 		if !isAttachment {
 			if _, ok := file.getHeader(HeaderContentID); !ok {
-				file.setHeader(HeaderContentID, fmt.Sprintf("<%s>", file.Name))
+				file.setHeader(HeaderContentID, fmt.Sprintf("<%s>", sanitizeFilename(file.Name)))
 			}
 		}
 		if mw.depth == 0 {
@@ -498,3 +498,33 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin
 		mw.bytesWritten += n
 	}
 }
+
+// sanitizeFilename sanitizes a given filename string by replacing specific unwanted characters with
+// an underscore ('_').
+//
+// This method replaces any control character and any special character that is problematic for
+// MIME headers and file systems with an underscore ('_') character.
+//
+// The following characters are replaced
+// - Any control character (US-ASCII < 32)
+// - ", /, :, <, >, ?, \, |, [DEL]
+//
+// Parameters:
+//   - input: A string of a filename that is supposed to be sanitized
+//
+// Returns:
+//   - A string representing the sanitized version of the filename
+func sanitizeFilename(input string) string {
+	var sanitized strings.Builder
+	for i := 0; i < len(input); i++ {
+		// We do not allow control characters in file names.
+		if input[i] < 32 || input[i] == 34 || input[i] == 47 || input[i] == 58 ||
+			input[i] == 60 || input[i] == 62 || input[i] == 63 || input[i] == 92 ||
+			input[i] == 124 || input[i] == 127 {
+			sanitized.WriteRune('_')
+			continue
+		}
+		sanitized.WriteByte(input[i])
+	}
+	return sanitized.String()
+}

msgwriter_test.go

@@ -304,6 +304,65 @@ func TestMsgWriter_addFiles(t *testing.T) {
 		charset: CharsetUTF8,
 		encoder: getEncoder(EncodingQP),
 	}
+	tests := []struct {
+		name     string
+		filename string
+		expect   string
+	}{
+		{"normal US-ASCII filename", "test.txt", "test.txt"},
+		{"normal US-ASCII filename with space", "test file.txt", "test file.txt"},
+		{"filename with new lines", "test\r\n.txt", "test__.txt"},
+		{"filename with disallowed character:\x22", "test\x22.txt", "test_.txt"},
+		{"filename with disallowed character:\x2f", "test\x2f.txt", "test_.txt"},
+		{"filename with disallowed character:\x3a", "test\x3a.txt", "test_.txt"},
+		{"filename with disallowed character:\x3c", "test\x3c.txt", "test_.txt"},
+		{"filename with disallowed character:\x3e", "test\x3e.txt", "test_.txt"},
+		{"filename with disallowed character:\x3f", "test\x3f.txt", "test_.txt"},
+		{"filename with disallowed character:\x5c", "test\x5c.txt", "test_.txt"},
+		{"filename with disallowed character:\x7c", "test\x7c.txt", "test_.txt"},
+		{"filename with disallowed character:\x7f", "test\x7f.txt", "test_.txt"},
+		{
+			"japanese characters filename", "添付ファイル.txt",
+			"=?UTF-8?q?=E6=B7=BB=E4=BB=98=E3=83=95=E3=82=A1=E3=82=A4=E3=83=AB.txt?=",
+		},
+		{
+			"simplified chinese characters filename", "测试附件文件.txt",
+			"=?UTF-8?q?=E6=B5=8B=E8=AF=95=E9=99=84=E4=BB=B6=E6=96=87=E4=BB=B6.txt?=",
+		},
+		{
+			"cyrillic characters filename", "Тестовый прикрепленный файл.txt",
+			"=?UTF-8?q?=D0=A2=D0=B5=D1=81=D1=82=D0=BE=D0=B2=D1=8B=D0=B9_=D0=BF=D1=80?= " +
+				"=?UTF-8?q?=D0=B8=D0=BA=D1=80=D0=B5=D0=BF=D0=BB=D0=B5=D0=BD=D0=BD=D1=8B?= " +
+				"=?UTF-8?q?=D0=B9_=D1=84=D0=B0=D0=B9=D0=BB.txt?=",
+		},
+	}
+	for _, tt := range tests {
+		t.Run("addFile with filename sanitization: "+tt.name, func(t *testing.T) {
+			buffer := bytes.NewBuffer(nil)
+			msgwriter.writer = buffer
+			message := testMessage(t)
+			message.AttachFile("testdata/attachment.txt", WithFileName(tt.filename))
+			msgwriter.writeMsg(message)
+			if msgwriter.err != nil {
+				t.Errorf("msgWriter failed to write: %s", msgwriter.err)
+			}
+
+			var ctExpect string
+			cdExpect := fmt.Sprintf(`Content-Disposition: attachment; filename="%s"`, tt.expect)
+			switch runtime.GOOS {
+			case "freebsd":
+				ctExpect = fmt.Sprintf(`Content-Type: application/octet-stream; name="%s"`, tt.expect)
+			default:
+				ctExpect = fmt.Sprintf(`Content-Type: text/plain; charset=utf-8; name="%s"`, tt.expect)
+			}
+			if !strings.Contains(buffer.String(), ctExpect) {
+				t.Errorf("expected content-type: %q, got: %q", ctExpect, buffer.String())
+			}
+			if !strings.Contains(buffer.String(), cdExpect) {
+				t.Errorf("expected content-disposition: %q, got: %q", cdExpect, buffer.String())
+			}
+		})
+	}
 	t.Run("message with a single file attached", func(t *testing.T) {
 		buffer := bytes.NewBuffer(nil)
 		msgwriter.writer = buffer
@@ -676,3 +735,33 @@ func TestMsgWriter_writeBody(t *testing.T) {
 		}
 	})
 }
+
+func TestMsgWriter_sanitizeFilename(t *testing.T) {
+	tests := []struct {
+		given string
+		want  string
+	}{
+		{"test.txt", "test.txt"},
+		{"test file.txt", "test file.txt"},
+		{"test\\ file.txt", "test_ file.txt"},
+		{`"test" file.txt`, "_test_ file.txt"},
+		{`test	file	.txt`, "test_file_.txt"},
+		{"test\r\nfile.txt", "test__file.txt"},
+		{"test\x22file.txt", "test_file.txt"},
+		{"test\x2ffile.txt", "test_file.txt"},
+		{"test\x3afile.txt", "test_file.txt"},
+		{"test\x3cfile.txt", "test_file.txt"},
+		{"test\x3efile.txt", "test_file.txt"},
+		{"test\x3ffile.txt", "test_file.txt"},
+		{"test\x5cfile.txt", "test_file.txt"},
+		{"test\x7cfile.txt", "test_file.txt"},
+		{"test\x7ffile.txt", "test_file.txt"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.given+"=>"+tt.want, func(t *testing.T) {
+			if got := sanitizeFilename(tt.given); got != tt.want {
+				t.Errorf("sanitizeFilename failed, expected: %q, got: %q", tt.want, got)
+			}
+		})
+	}
+}