Skip to content

Commit

Permalink
Fix tests and boundary condition (#410)
Browse files Browse the repository at this point in the history 
Fix two failing tests
Write a bunch more tests to expand coverage and check for errors
Fix a bug where we can have no response and trigger a success
  • Loading branch information
@Seanstoppable
Seanstoppable authored May 22, 2024
1 parent 4d181ea commit 990c9c3
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 64 deletions.
2 changes: 1 addition & 1 deletion lib/http/transport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1553,7 +1553,7 @@ func (pc *persistConn) readLoop() {
closeErr = err
}

if err != nil && (!pc.sawEOF || resp == nil) {
if err != nil && (!pc.sawEOF || resp == nil || resp.Status == "") {
if pc.readLimit <= 0 {
err = fmt.Errorf("net/http: server response headers exceeded %d bytes; aborted", pc.maxHeaderResponseSize())
}
Expand Down
208 changes: 145 additions & 63 deletions modules/http/http_readlimit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/zmap/zcrypto/tls"
"github.com/zmap/zgrab2"
"github.com/zmap/zgrab2/lib/http"
)

// BEGIN Taken from handshake_server_test.go -- certs for TLS server
Expand Down Expand Up @@ -107,7 +108,13 @@ func (cfg *readLimitTestConfig) runFakeHTTPServer(t *testing.T) {
}

head := "HTTP/1.0 200 OK\r\nBogus-Header: X"
if cfg.customHeader != nil {
head = *cfg.customHeader
}
headSuffix := fmt.Sprintf("\r\nContent-Length: %d\r\n\r\n", cfg.bodySize)
if cfg.customSuffix != nil {
headSuffix = *cfg.customSuffix
}
size := cfg.headerSize - len(head) - len(headSuffix)
if size < 0 {
t.Fatalf("Header size %d too small: must be at least %d bytes", cfg.headerSize, len(head)+len(headSuffix))
Expand Down Expand Up @@ -191,13 +198,20 @@ type readLimitTestConfig struct {

// If set, the error returned by the scan must contain this.
expectedError string

// If set, return a custom header
customHeader *string

customSuffix *string
}

const (
readLimitTestConfigHTTPBasePort = 0x7f7f
readLimitTestConfigHTTPSBasePort = 0x7bbc
)

func adr(s string) *string {return &s}

var readLimitTestConfigs = map[string]*readLimitTestConfig{
// The socket truncates the connection while reading the body. To the client it looks as if the
// server closed the connection prior to sending Content-Length bytes; the result is success,
Expand All @@ -219,84 +233,140 @@ var readLimitTestConfigs = map[string]*readLimitTestConfig{
// and the truncated body.
// maxReadSize > headerSize + bodySize > bodySize > maxBodySize
"truncate_body": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 1,
maxBodySize: 2048,
maxReadSize: 8192,
headerSize: 64,
bodySize: 4096,
tls: false,
port: readLimitTestConfigHTTPBasePort + 1,
maxBodySize: 2048,
maxReadSize: 8192,
headerSize: 64,
bodySize: 4096,
expectedStatus: zgrab2.SCAN_SUCCESS,
},
"tls_truncate_body": {
tls: true,
port: readLimitTestConfigHTTPSBasePort + 1,
maxBodySize: 2048,
maxReadSize: 8192,
headerSize: 64,
bodySize: 4096,
tls: true,
port: readLimitTestConfigHTTPSBasePort + 1,
maxBodySize: 2048,
maxReadSize: 8192,
headerSize: 64,
bodySize: 4096,
expectedStatus: zgrab2.SCAN_SUCCESS,
},

// The socket truncates the connection while reading the headers. The result isn't a valid HTTP
// response, so the library returns an unexpected EOF error.
// The socket truncates the connection while reading the headers. The result isn't a completely valid HTTP
// response, but we capture the output regardless
// headerSize > maxReadSize
"truncate_read_header": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 2,
maxBodySize: 1024,
maxReadSize: 2048,
headerSize: 3072,
bodySize: 8,
expectedError: "unexpected EOF",
expectedStatus: zgrab2.SCAN_UNKNOWN_ERROR,
tls: false,
port: readLimitTestConfigHTTPBasePort + 2,
maxBodySize: 1024,
maxReadSize: 2048,
headerSize: 3072,
bodySize: 0,
expectedStatus: zgrab2.SCAN_SUCCESS,
},
"tls_truncate_read_header": {
tls: true,
port: readLimitTestConfigHTTPSBasePort + 2,
maxBodySize: 1024,
maxReadSize: 2048,
headerSize: 3072,
bodySize: 8,
expectedError: "unexpected EOF",
tls: true,
port: readLimitTestConfigHTTPSBasePort + 2,
maxBodySize: 1024,
maxReadSize: 2048,
headerSize: 3072,
bodySize: 0,
expectedStatus: zgrab2.SCAN_SUCCESS,
},

// The socket truncates the connection while reading the status code. The result isn't a valid HTTP
// response
// headerSize > maxReadSize
"invalid_status_code": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 2,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
customHeader: adr("HTTP/1.0 200"),
expectedError: "malformed HTTP status code",
expectedStatus: zgrab2.SCAN_UNKNOWN_ERROR,
},
"tls_invalid_status_code": {
tls: true,
port: readLimitTestConfigHTTPSBasePort + 2,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
customHeader: adr("HTTP/1.0 200"),
expectedError: "malformed HTTP status code",
expectedStatus: zgrab2.SCAN_UNKNOWN_ERROR,
},

"invalid_no_status": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 2,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
customHeader: adr(""),
customSuffix: adr(""),
expectedError: "malformed HTTP response",
expectedStatus: zgrab2.SCAN_UNKNOWN_ERROR,
},

"invalid_response": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 2,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
customHeader: adr(""),
expectedError: "malformed HTTP response",
expectedStatus: zgrab2.SCAN_UNKNOWN_ERROR,
},

"invalid_low_read_limit": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 2,
maxBodySize: 8192,
maxReadSize: 1,
headerSize: 1024,
bodySize: 1024,
expectedError: "malformed HTTP response",
expectedStatus: zgrab2.SCAN_UNKNOWN_ERROR,
},

// Happy case. None of the limits are hit.
// maxReadSize >= maxBodySize > bodySize + headerSize
"happy_case": {
tls: false,
port: readLimitTestConfigHTTPBasePort + 3,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
tls: false,
port: readLimitTestConfigHTTPBasePort + 3,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
expectedStatus: zgrab2.SCAN_SUCCESS,
},
"tls_happy_case": {
tls: true,
port: readLimitTestConfigHTTPSBasePort + 3,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
tls: true,
port: readLimitTestConfigHTTPSBasePort + 3,
maxBodySize: 8192,
maxReadSize: 8192,
headerSize: 1024,
bodySize: 1024,
expectedStatus: zgrab2.SCAN_SUCCESS,
},
}

// Try to get the HTTP body from a result; otherwise return the empty string.
func getBody(result interface{}) string {
func getResponse(result interface{}) *http.Response {
if result == nil {
return ""
return nil
}
httpResult, ok := result.(*Results)
if !ok {
return ""
}
response := httpResult.Response
if response == nil {
return ""
return nil
}
return response.BodyText
return httpResult.Response
}

// Run a single test with the given configuration.
Expand All @@ -307,31 +377,43 @@ func (cfg *readLimitTestConfig) runTest(t *testing.T, testName string) {
IP: net.ParseIP("127.0.0.1"),
}
status, ret, err := scanner.Scan(target)
response := getResponse(ret)

if status != cfg.expectedStatus {
t.Errorf("Wrong status: expected %s, got %s", cfg.expectedStatus, status)
t.Errorf("Wrong status: expected %s, got %s with %+v", cfg.expectedStatus, status, response)
}
if err != nil {
if !strings.Contains(err.Error(), cfg.expectedError) {
t.Errorf("Wrong error: expected %s, got %s", err.Error(), cfg.expectedError)
t.Errorf("Wrong error: expected %s, got %s", cfg.expectedError, err.Error())
}
} else if len(cfg.expectedError) > 0 {
t.Errorf("Expected error '%s' but got none", cfg.expectedError)
}
if cfg.expectedStatus == zgrab2.SCAN_SUCCESS {
body := getBody(ret)
if body == "" {
t.Errorf("Expected success, but got no body")
} else {
if len(body) > cfg.maxBodySize || len(body) > cfg.maxReadSize {
t.Errorf("Body exceeds max size: len(body)=%d; maxBodySize=%d, maxReadSize=%d", len(body), cfg.maxBodySize, cfg.maxReadSize)
}
if !cfg.tls {
if len(body)+cfg.headerSize > cfg.maxReadSize {
t.Errorf("Body and header exceed max read size: len(body)=%d, headerSize=%d, maxReadSize=%d", len(body), cfg.headerSize, cfg.maxReadSize)
}
}
if response == nil {
t.Errorf("Expected response, but got none")
}

statusCode := response.Status
if statusCode != "200 OK" {
t.Errorf("Expected status %s, but got %s", "200 OK", statusCode)
}

body := response.BodyText
if body == "" {
if cfg.bodySize != 0 {
t.Errorf("Expected success, but got no body")
}
} else {
if len(body) > cfg.maxBodySize || len(body) > cfg.maxReadSize {
t.Errorf("Body exceeds max size: len(body)=%d; maxBodySize=%d, maxReadSize=%d", len(body), cfg.maxBodySize, cfg.maxReadSize)
}
if !cfg.tls {
if len(body)+cfg.headerSize > cfg.maxReadSize {
t.Errorf("Body and header exceed max read size: len(body)=%d, headerSize=%d, maxReadSize=%d", len(body), cfg.headerSize, cfg.maxReadSize)
}
}
}
}
}

Expand Down

0 comments on commit 990c9c3

Please sign in to comment.