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

Ecubit/fix false positives #262

Merged
merged 12 commits into from Jun 1, 2020
Merged

Ecubit/fix false positives #262

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5208cc5
telnet module should not return success when it finds something other…
May 20, 2020
d3e2899
telnet module should not return success when it finds something other…
elliotcubit May 21, 2020
b68e503
Adds verification step for POP3 banners
elliotcubit May 21, 2020
e663226
Merge branch 'ecubit/fix-false-positives' of https://github.com/ellio…
elliotcubit May 21, 2020
3d4e783
Whoops :)
elliotcubit May 21, 2020
1316e56
Add validation for IMAP banners & fix some formatting problems
elliotcubit May 21, 2020
8f645d6
Verify SMTP banners exist and are successful
elliotcubit May 21, 2020
e4218eb
Add check for is_dnp3 flag which seems to be working as expected
elliotcubit May 21, 2020
532a496
Fix dropping SCAN_APPLICATION_ERROR in IMAP
elliotcubit May 28, 2020
d6c83fd
Fix dropping SCAN_APPLICATION_ERROR in POP3
elliotcubit May 28, 2020
734fdca
Fix dropping SCAN_APPLICATION_ERROR in SMTP
elliotcubit May 28, 2020
7754d3a
Add protocol and blacklist indicators to email protocols
elliotcubit May 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/weppos/publicsuffix-go v0.4.0 h1:YSnfg3V65LcCFKtIGKGoBhkyKolEd0hlipcXaOjdnQw=
github.com/weppos/publicsuffix-go v0.4.0/go.mod h1:z3LCPQ38eedDQSwmsSRW4Y7t2L8Ln16JPQ02lHAdn5k=
github.com/zmap/rc2 v0.0.0-20131011165748-24b9757f5521 h1:kKCF7VX/wTmdg2ZjEaqlq99Bjsoiz7vH6sFniF/vI4M=
Expand Down
4 changes: 3 additions & 1 deletion modules/dnp3/dnp3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/binary"
"io"
"net"
"errors"

"github.com/zmap/zgrab2"
)
Expand Down Expand Up @@ -73,9 +74,10 @@ func GetDNP3Banner(logStruct *DNP3Log, connection net.Conn) (err error) {
if len(data) >= LINK_MIN_HEADER_LENGTH && binary.BigEndian.Uint16(data[0:2]) == LINK_START_FIELD {
logStruct.IsDNP3 = true
logStruct.RawResponse = data
return nil
}

return nil
return zgrab2.NewScanError(zgrab2.SCAN_PROTOCOL_ERROR, errors.New("Invalid response for DNP3"))
}

func makeLinkStatusRequest(dstAddress uint16) []byte {
Expand Down
28 changes: 23 additions & 5 deletions modules/imap/imap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package imap
import (
"net"
"regexp"
"strings"
"errors"
"io"

"github.com/zmap/zgrab2"
)
Expand All @@ -17,15 +20,30 @@ type Connection struct {
Conn net.Conn
}

// Verify banner begins with a valid IMAP response and handle it
func VerifyIMAPContents(n int, ret []byte) (string, error) {
s := string(ret[:n])
if strings.HasPrefix(s, "* OK"){
return s, nil
}
if strings.HasPrefix(s, "* NO"){
return s, zgrab2.NewScanError(zgrab2.SCAN_APPLICATION_ERROR, errors.New("IMAP reported error"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't actually an error, at least for us. Since the goal of zgrab2 is to identify services, receiving a well-formed error response is a good thing, as it is a positive indication that (in this case) we are talking to an imap server. (This also applies to the * BAD case, as well).

There is also * PREAUTH and * BYE

}
if strings.HasPrefix(s, "* BAD"){
return s, zgrab2.NewScanError(zgrab2.SCAN_UNKNOWN_ERROR, errors.New("IMAP request was malformed"))
}
return s, zgrab2.NewScanError(zgrab2.SCAN_PROTOCOL_ERROR, errors.New("Invalid response for IMAP"))
}

// ReadResponse reads from the connection until it matches the imapEndRegex. Copied from the original zgrab.
// TODO: Catch corner cases, parse out success/error character.
// TODO: Catch corner cases
func (conn *Connection) ReadResponse() (string, error) {
ret := make([]byte, readBufferSize)
n, err := zgrab2.ReadUntilRegex(conn.Conn, ret, imapStatusEndRegex)
if err != nil {
return "", nil
if err != nil && err != io.EOF && !zgrab2.IsTimeoutError(err) {
return "", err
}
return string(ret[0:n]), nil
return VerifyIMAPContents(n, ret)
}

// SendCommand sends a command, followed by a CRLF, then wait for / read the server's response.
Expand All @@ -34,4 +52,4 @@ func (conn *Connection) SendCommand(cmd string) (string, error) {
return "", err
}
return conn.ReadResponse()
}
}
29 changes: 24 additions & 5 deletions modules/pop3/pop3.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
// Scanner for POP3 protocol
// https://www.ietf.org/rfc/rfc1939.txt

package pop3

import (
"net"
"regexp"
"errors"
"strings"
"io"

"github.com/zmap/zgrab2"
)
Expand All @@ -17,15 +23,28 @@ type Connection struct {
Conn net.Conn
}

// Verifies that a POP3 banner begins with a valid status indicator
func VerifyPOP3Contents(n int, ret []byte) (string, error) {
s := string(ret[0:n])
if strings.HasPrefix(s, "+OK "){
return s, nil
}
if strings.HasPrefix(s, "+ERR "){
return s, zgrab2.NewScanError(zgrab2.SCAN_APPLICATION_ERROR, errors.New("POP3 Reported Error"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment as for imap; this is actually a success case

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the contents of the modbus scanner, it seems that SCAN_APPLICATION_ERROR is a success case - indicating that a valid response was received but the server reported back an error in the final ScanResult

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I just realized that string should be "-ERR" regardless

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SCAN_APPLICATION_ERROR can be treated as a "success" error case; the real indication is if the scanner returns nil for the results or not. So it would be OK in this and the IMAP module to return SCAN_APPLICATION_ERROR here, except that in both of them scanner.go will omit results.

For example, from pop3/scanner.go:

    banner, err := conn.ReadResponse()
    if err != nil {
        return zgrab2.TryGetScanStatus(err), nil, err
    }

}
return s, zgrab2.NewScanError(zgrab2.SCAN_PROTOCOL_ERROR, errors.New("Invalid response for POP3"))
}

// ReadResponse reads from the connection until it matches the pop3EndRegex. Copied from the original zgrab.
// TODO: Catch corner cases, parse out success/error character.
// TODO: Catch corner cases
func (conn *Connection) ReadResponse() (string, error) {
ret := make([]byte, readBufferSize)
n, err := zgrab2.ReadUntilRegex(conn.Conn, ret, pop3EndRegex)
if err != nil {
return "", nil
// Don't quit for timeouts since we might have gotten relevant data still
if err != nil && err != io.EOF && !zgrab2.IsTimeoutError(err) {
return "", err
}
return string(ret[0:n]), nil
return VerifyPOP3Contents(n, ret)
}

// SendCommand sends a command, followed by a CRLF, then wait for / read the server's response.
Expand All @@ -34,4 +53,4 @@ func (conn *Connection) SendCommand(cmd string) (string, error) {
return "", err
}
return conn.ReadResponse()
}
}
2 changes: 1 addition & 1 deletion modules/smtp/scanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import (
)

// ErrInvalidResponse is returned when the server returns an invalid or unexpected response.
var ErrInvalidResponse = errors.New("invalid response")
var ErrInvalidResponse = zgrab2.NewScanError(zgrab2.SCAN_PROTOCOL_ERROR, errors.New("Invalid response for SMTP"))

// ScanResults instances are returned by the module's Scan function.
type ScanResults struct {
Expand Down
23 changes: 19 additions & 4 deletions modules/smtp/smtp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package smtp
import (
"net"
"regexp"
"fmt"
"io"

"github.com/zmap/zgrab2"
)
Expand All @@ -18,15 +20,28 @@ type Connection struct {
Conn net.Conn
}

// Verify that an SMTP code was returned, and that it is a successful one!
func VerifySMTPContents(n int, ret []byte) (string, error){
s := string(ret[:n])
code, err := getSMTPCode(s)
if err != nil {
return s, err
}
if code < 200 || code >= 300 {
return s, zgrab2.NewScanError(zgrab2.SCAN_APPLICATION_ERROR, fmt.Errorf("SMTP returned error code %d", code))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment; if we wanted to vet that it is a valid code, anything returned as a code from the SMTP RFC would qualify as success. However, I wonder if we even need to be that strict (in case there are non-standard extensions out there)?

}
return s, nil
}

// ReadResponse reads from the connection until it matches the smtpEndRegex. Copied from the original zgrab.
// TODO: Catch corner cases, parse out response code.
// TODO: Catch corner cases
func (conn *Connection) ReadResponse() (string, error) {
ret := make([]byte, readBufferSize)
n, err := zgrab2.ReadUntilRegex(conn.Conn, ret, smtpEndRegex)
if err != nil {
return "", nil
if err != nil && err != io.EOF && !zgrab2.IsTimeoutError(err) {
return "", err
}
return string(ret[0:n]), nil
return VerifySMTPContents(n, ret)
}

// SendCommand sends a command, followed by a CRLF, then wait for / read the server's response.
Expand Down
4 changes: 4 additions & 0 deletions modules/telnet/telnet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ func GetTelnetBanner(logStruct *TelnetLog, conn net.Conn, maxReadSize int) (err
if err != nil && err != io.EOF && !zgrab2.IsTimeoutError(err) {
return err
}
// Make sure it is a telnet banner
if !logStruct.isTelnet() {
return zgrab2.NewScanError(zgrab2.SCAN_PROTOCOL_ERROR, errors.New("Invalid response for Telnet"))
}
return nil
}

Expand Down