Fix multiple bugs found in code review

blacklists.go

@@ -12,7 +12,6 @@ var dnsbllistip = []string{"ix.dnsbl.manitu.net.",
 	"truncate.gbudb.net.",
 	"dnsbl.dronebl.org.",
 	"rblspamassassin.interserver.net.",
-	"ix.dnsbl.manitu.net.",
 	"bl.spamcop.net.",
 	"dnsbl-2.uceprotect.net.",
 	"spam.dnsbl.sorbs.net.",
@@ -63,7 +62,8 @@ func checkdnsblName(domainname string, dnsServer string) (map[string]string, map
 
 		in, _, err := c.Exchange(m, dnsServer+":53")
 		if err != nil {
-			println(err.Error())
+			ErrorLogger.Println(err)
+			continue
 		}
 
 		if len(in.Answer) == 0 {

checkversion.go

@@ -10,18 +10,17 @@ type MXCVersion struct {
 }
 
 func getLatestVersion() error {
-	var err error
 	url := "https://api.github.com/repos/steffenfritz/mxcheck/releases/latest"
 	resp, err := http.Get(url)
 	if err != nil {
-		InfoLogger.Fatalln(err)
+		return err
 	}
+	defer resp.Body.Close()
 
 	var mxcv MXCVersion
 	jsondec := json.NewDecoder(resp.Body)
-	err = jsondec.Decode(&mxcv)
-	if err != nil {
-		InfoLogger.Fatalln(err)
+	if err = jsondec.Decode(&mxcv); err != nil {
+		return err
 	}
 
 	if mxcv.Tag_name == Version {
@@ -32,5 +31,5 @@ func getLatestVersion() error {
 		InfoLogger.Println("Available version: " + mxcv.Tag_name)
 	}
 
-	return err
+	return nil
 }

connect_test.go

@@ -18,7 +18,12 @@ func Test_portScan(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := portScan(tt.args.targetHost); !reflect.DeepEqual(got[0], tt.want) {
+			got := portScan(tt.args.targetHost)
+			if len(got) == 0 {
+				t.Errorf("portScan() = [], want %v", tt.want)
+				return
+			}
+			if !reflect.DeepEqual(got[0], tt.want) {
 				t.Errorf("portScan() = %v, want %v", got[0], tt.want)
 			}
 		})

dns.go

@@ -62,7 +62,7 @@ func getA(targetHostName string, dnsServer string) (string, error) {
 		a = t.A.String()
 		// We check for a second ip address when the first entry is also an mx entry.
 		// We also have to update the targetHost because of the cwrt test in openrelay
-	} else {
+	} else if len(in.Answer) > 1 {
 		if t, ok := in.Answer[1].(*dns.A); ok {
 			a = t.A.String()
 		}
@@ -194,27 +194,27 @@ func getDKIM(selector string, targetHostName string, dnsServer string) (dkim, er
 						dkim.version = "1"
 						for _, partDKIM := range dkimSplit {
 							if strings.HasPrefix(partDKIM, "g=") {
-								dkim.granularity = strings.TrimRight(strings.Split(partDKIM, "=")[1], ";")
+								dkim.granularity = strings.TrimRight(strings.SplitN(partDKIM, "=", 2)[1], ";")
 								continue
 							}
 							if strings.HasPrefix(partDKIM, "h=") {
-								dkim.accepAlgo = strings.TrimRight(strings.Split(partDKIM, "=")[1], ";")
+								dkim.accepAlgo = strings.TrimRight(strings.SplitN(partDKIM, "=", 2)[1], ";")
 								continue
 							}
 							if strings.HasPrefix(partDKIM, "k=") {
-								dkim.keyType = strings.TrimRight(strings.Split(partDKIM, "=")[1], ";")
+								dkim.keyType = strings.TrimRight(strings.SplitN(partDKIM, "=", 2)[1], ";")
 								continue
 							}
 							if strings.HasPrefix(partDKIM, "n=") {
-								dkim.noteField = strings.TrimRight(strings.Split(partDKIM, "=")[1], ";")
+								dkim.noteField = strings.TrimRight(strings.SplitN(partDKIM, "=", 2)[1], ";")
 								continue
 							}
 							if strings.HasPrefix(partDKIM, "p=") {
-								dkim.publicKey = strings.TrimRight(strings.Split(partDKIM, "=")[1], ";")
+								dkim.publicKey = strings.TrimRight(strings.SplitN(partDKIM, "=", 2)[1], ";")
 								continue
 							}
 							if strings.HasPrefix(partDKIM, "t=") {
-								dkim.testing = strings.TrimRight(strings.Split(partDKIM, "=")[1], ";")
+								dkim.testing = strings.TrimRight(strings.SplitN(partDKIM, "=", 2)[1], ";")
 								continue
 							}
 						}

main.go

@@ -422,11 +422,10 @@ func main() {
 					} else {
 						InfoLogger.Println(Cyan("SMTPS not supported"))
 					}
-
-					runresult.mxresults = append(runresult.mxresults, singlemx)
-					println()
 				}
 			}
+			runresult.mxresults = append(runresult.mxresults, singlemx)
+			println()
 		}
 	}
 

mtasts.go

@@ -4,6 +4,7 @@ import (
 	"io"
 	"net/http"
 	"strings"
+	"time"
 )
 
 // mtastsuri is the fixed uri suffix defined in rfc8461
@@ -23,20 +24,27 @@ type mtaststxt struct {
 //mtasts checks if mtasts is wanted and possible
 func mtasts(targetHostname string) (mtaststxt, error) {
 	var mtaststxt mtaststxt
-	resp, err := http.Get(mtastsprefix + targetHostname + mtastsuri)
+	client := &http.Client{Timeout: 10 * time.Second}
+	resp, err := client.Get(mtastsprefix + targetHostname + mtastsuri)
 	if err != nil {
 		return mtaststxt, err
 	}
 	defer resp.Body.Close()
 	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return mtaststxt, err
+	}
 
 	mtastssplit := strings.Split(string(body), "\n")
+	if len(mtastssplit) < 3 {
+		return mtaststxt, nil
+	}
 	mtaststxt.version = mtastssplit[0]
 	mtaststxt.mode = mtastssplit[1]
 	mtaststxt.maxAge = mtastssplit[2]
-	for _, mxentry := range mtastssplit[2:] {
+	for _, mxentry := range mtastssplit[3:] {
 		mtaststxt.mx = append(mtaststxt.mx, mxentry)
 	}
 
-	return mtaststxt, err
+	return mtaststxt, nil
 }

openrelay.go

@@ -4,7 +4,6 @@ import (
 	"bufio"
 	"crypto/tls"
 	"fmt"
-	"log"
 	"net"
 	"net/smtp"
 	"strings"
@@ -42,17 +41,17 @@ func openRelay(mailFrom string, mailTo string, targetHost string, targetPort str
 
 	// Print server string. You can trust the server string, but you shouldn't...
 	conn, err := net.DialTimeout("tcp", targetHost+":"+targetPort, 15*time.Second)
-	defer conn.Close()
-
 	if err != nil {
 		or.serverstring = "Could not read banner: " + err.Error()
 	} else {
+		defer conn.Close()
 		buf := bufio.NewReader(conn)
 		bannerbytes, err := buf.ReadBytes('\n')
 		if err != nil {
-			log.Fatalf("ee Fatal error: %s", err.Error())
+			ErrorLogger.Println("Error reading banner: " + err.Error())
+		} else {
+			or.serverstring = string(bannerbytes)
 		}
-		or.serverstring = string(bannerbytes)
 	}
 
 	// set default TLS config

output.go

@@ -22,77 +22,159 @@ func writeTSV(targetHostName string, runresult runresult, blacklist bool) error
 	if err != nil {
 		return err
 	}
+	defer fd.Close()
 	tsv := NewTSVWriter(fd)
-	err = tsv.Write([]string{"Test Date", runresult.testdate})
-	err = tsv.Write([]string{"Target Domain Name", runresult.targetdomainname})
-	err = tsv.Write([]string{"DNS Server", runresult.dnsserver})
-	err = tsv.Write([]string{"MailFrom", runresult.mailfrom})
-	err = tsv.Write([]string{"MailTo", runresult.mailto})
+	if err = tsv.Write([]string{"Test Date", runresult.testdate}); err != nil {
+		return err
+	}
+	if err = tsv.Write([]string{"Target Domain Name", runresult.targetdomainname}); err != nil {
+		return err
+	}
+	if err = tsv.Write([]string{"DNS Server", runresult.dnsserver}); err != nil {
+		return err
+	}
+	if err = tsv.Write([]string{"MailFrom", runresult.mailfrom}); err != nil {
+		return err
+	}
+	if err = tsv.Write([]string{"MailTo", runresult.mailto}); err != nil {
+		return err
+	}
 	if !runresult.dkimresult.dkimset {
-		err = tsv.Write([]string{"DKIM Set", "false or not checked"})
+		if err = tsv.Write([]string{"DKIM Set", "false or not checked"}); err != nil {
+			return err
+		}
 	} else {
-		err = tsv.Write([]string{"DKIM Set", "true"})
-		err = tsv.Write([]string{"DKIM DNS Entry", runresult.dkimresult.domain})
-		err = tsv.Write([]string{"DKIM DNS Version", runresult.dkimresult.version})
-		err = tsv.Write([]string{"DKIM Key Type", runresult.dkimresult.keyType})
-		err = tsv.Write([]string{"DKIM Accepted Algorithm", runresult.dkimresult.accepAlgo})
-		err = tsv.Write([]string{"DKIM Granularity", runresult.dkimresult.granularity})
-		err = tsv.Write([]string{"DKIM Note", runresult.dkimresult.noteField})
-		err = tsv.Write([]string{"DKIM Public Key", runresult.dkimresult.publicKey})
+		if err = tsv.Write([]string{"DKIM Set", "true"}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM DNS Entry", runresult.dkimresult.domain}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM DNS Version", runresult.dkimresult.version}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM Key Type", runresult.dkimresult.keyType}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM Accepted Algorithm", runresult.dkimresult.accepAlgo}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM Granularity", runresult.dkimresult.granularity}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM Note", runresult.dkimresult.noteField}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"DKIM Public Key", runresult.dkimresult.publicKey}); err != nil {
+			return err
+		}
 	}
 
-	err = tsv.Write([]string{"DMARC Set", strconv.FormatBool(runresult.dmarcset)})
+	if err = tsv.Write([]string{"DMARC Set", strconv.FormatBool(runresult.dmarcset)}); err != nil {
+		return err
+	}
 	if runresult.dmarcset {
-		err = tsv.Write([]string{"DMARC Entry", runresult.dmarcfull})
-
+		if err = tsv.Write([]string{"DMARC Entry", runresult.dmarcfull}); err != nil {
+			return err
+		}
 	}
 
 	for i, mxentry := range runresult.mxresults {
-		err = tsv.Write([]string{"MX Entry Seq Number ", strconv.Itoa(i)})
-		err = tsv.Write([]string{"MX Entry DNS", mxentry.mxentry})
-		err = tsv.Write([]string{"IP Address", mxentry.ipaddr})
-		err = tsv.Write([]string{"AS Number", strconv.Itoa(mxentry.asnum)})
-		err = tsv.Write([]string{"AS Country", mxentry.ascountry})
-		err = tsv.Write([]string{"Server string", mxentry.serverstring})
+		if err = tsv.Write([]string{"MX Entry Seq Number ", strconv.Itoa(i)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"MX Entry DNS", mxentry.mxentry}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"IP Address", mxentry.ipaddr}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"AS Number", strconv.Itoa(mxentry.asnum)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"AS Country", mxentry.ascountry}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"Server string", mxentry.serverstring}); err != nil {
+			return err
+		}
 		portlist := ""
 		for _, port := range mxentry.openports {
 			portlist += port + " "
 		}
-		err = tsv.Write([]string{"Open Ports", portlist})
-		err = tsv.Write([]string{"PTR Entry", mxentry.ptrentry})
-		err = tsv.Write([]string{"PTR Match", strconv.FormatBool(mxentry.ptrmatch)})
-		err = tsv.Write([]string{"SPF Set", strconv.FormatBool(mxentry.spfset)})
-		err = tsv.Write([]string{"MTA-STS Set", strconv.FormatBool(mxentry.stsset)})
-		err = tsv.Write([]string{"STARTTLS PORT 25 Supported", strconv.FormatBool(mxentry.starttls)})
-		err = tsv.Write([]string{"STARTTLS TLS Version", mxentry.starttlsversion})
-		err = tsv.Write([]string{"Certificate Valid", strconv.FormatBool(mxentry.tlscertvalid)})
+		if err = tsv.Write([]string{"Open Ports", portlist}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"PTR Entry", mxentry.ptrentry}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"PTR Match", strconv.FormatBool(mxentry.ptrmatch)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"SPF Set", strconv.FormatBool(mxentry.spfset)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"MTA-STS Set", strconv.FormatBool(mxentry.stsset)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"STARTTLS PORT 25 Supported", strconv.FormatBool(mxentry.starttls)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"STARTTLS TLS Version", mxentry.starttlsversion}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"Certificate Valid", strconv.FormatBool(mxentry.tlscertvalid)}); err != nil {
+			return err
+		}
 		if slices.Contains(mxentry.openports, "465") {
-			err = tsv.Write([]string{"TLS Version PORT 465 ", mxentry.tlsversion})
-		}
-		err = tsv.Write([]string{"VRFY Supported", strconv.FormatBool(mxentry.vrfysupport)})
-		err = tsv.Write([]string{"SMTP Smuggling Vulnerability", strconv.FormatBool(mxentry.smugglevuln)})
-		err = tsv.Write([]string{"SMTP Smuggling Response", mxentry.smuggleresp})
-		err = tsv.Write([]string{"Fake Sender Accepted", strconv.FormatBool(mxentry.fakesender)})
-		err = tsv.Write([]string{"Fake Recipient Accepted", strconv.FormatBool(mxentry.fakercpt)})
-		err = tsv.Write([]string{"Open Relay", strconv.FormatBool(mxentry.openrelay)})
+			if err = tsv.Write([]string{"TLS Version PORT 465 ", mxentry.tlsversion}); err != nil {
+				return err
+			}
+		}
+		if err = tsv.Write([]string{"VRFY Supported", strconv.FormatBool(mxentry.vrfysupport)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"SMTP Smuggling Vulnerability", strconv.FormatBool(mxentry.smugglevuln)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"SMTP Smuggling Response", mxentry.smuggleresp}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"Fake Sender Accepted", strconv.FormatBool(mxentry.fakesender)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"Fake Recipient Accepted", strconv.FormatBool(mxentry.fakercpt)}); err != nil {
+			return err
+		}
+		if err = tsv.Write([]string{"Open Relay", strconv.FormatBool(mxentry.openrelay)}); err != nil {
+			return err
+		}
 	}
 
 	if blacklist {
 		for bldns, blacklistresult := range runresult.bldnsnamelisted {
-			err = tsv.Write([]string{"Blacklist " + bldns + " lists ", blacklistresult})
+			if err = tsv.Write([]string{"Blacklist " + bldns + " lists ", blacklistresult}); err != nil {
+				return err
+			}
 		}
 		for bldns, blacklistresult := range runresult.bldnsnamenotlisted {
-			err = tsv.Write([]string{"Blacklist " + bldns + " does not list ", blacklistresult})
+			if err = tsv.Write([]string{"Blacklist " + bldns + " does not list ", blacklistresult}); err != nil {
+				return err
+			}
 		}
 		for bldns, blacklistresult := range runresult.bldnsiplisted {
-			err = tsv.Write([]string{"Blacklist " + bldns + " lists ", blacklistresult})
+			if err = tsv.Write([]string{"Blacklist " + bldns + " lists ", blacklistresult}); err != nil {
+				return err
+			}
 		}
 		for bldns, blacklistresult := range runresult.bldnsipnotlisted {
-			err = tsv.Write([]string{"Blacklist " + bldns + " does not list ", blacklistresult})
+			if err = tsv.Write([]string{"Blacklist " + bldns + " does not list ", blacklistresult}); err != nil {
+				return err
+			}
 		}
 	}
 
 	tsv.Flush()
 
-	return err
+	return tsv.Error()
 }