Skip to content

Commit

Permalink
fix huge bug with default variables
Browse files Browse the repository at this point in the history
  • Loading branch information
jptosso committed Aug 25, 2021
1 parent 2dbeb39 commit fc63d67
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 36 deletions.
15 changes: 5 additions & 10 deletions coraza.conf-recommended
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,6 @@ FL %{MULTIPART_FILE_LIMIT_EXCEEDED}'"
SecRule MULTIPART_UNMATCHED_BOUNDARY "@eq 1" \
"id:'200004',phase:2,t:none,log,deny,msg:'Multipart parser detected a possible unmatched boundary.'"


# PCRE Tuning
# We want to avoid a potential RegEx DoS condition
# **Not supported yet**
#
SecPcreMatchLimit 1000
SecPcreMatchLimitRecursion 1000

# Some internal errors will set flags in TX and we will need to look for these.
# All of these are prefixed with "MSC_". The following flags currently exist:
#
Expand Down Expand Up @@ -246,10 +238,13 @@ SecAuditLogParts ABIJDEFHZ
# Use a single file for logging. This is much easier to look at, but
# assumes that you will use the audit log only ocassionally.
#
#SecAuditLog Modsec /opt/coraza/var/log/coraza_audit.log 0600
#SecAuditLog Serial file=/opt/coraza/var/log/coraza_audit.log filemode=0600

# Concurrent logging:
SecAuditLog Concurrent /opt/coraza/var/audit/audit.log /opt/coraza/var/audit 0600 0600
SecAuditLog Concurrent file=/opt/coraza/var/audit/audit.log \
path=/opt/coraza/var/audit \
filemode=0600 \
dirmode=0600


# -- Miscellaneous -----------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion operators/geo_lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (o *GeoLookup) Evaluate(tx *engine.Transaction, value string) bool {
if err != nil {
return false
}
tx.GetCollection(engine.VARIABLE_GEO).Add("COUNTRY_CODE", record.Country.IsoCode)
tx.GetCollection(engine.VARIABLE_GEO).Set("COUNTRY_CODE", []string{record.Country.IsoCode})
//TODO:
// NOTE: US ONLY VARIABLES WON'T BE ADDED, also NA and SA will be replaced with AM because of reasons
// COUNTRY_CODE3: Up to three character country code.
Expand Down
46 changes: 23 additions & 23 deletions transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,12 @@ func (tx *Transaction) AddRequestHeader(key string, value string) {
if key == "content-type" {
val := strings.ToLower(value)
if val == "application/x-www-form-urlencoded" {
tx.GetCollection(VARIABLE_REQBODY_PROCESSOR).Add("", "URLENCODED")
tx.GetCollection(VARIABLE_REQBODY_PROCESSOR).Set("", []string{"URLENCODED"})
} else if strings.HasPrefix(val, "multipart/form-data") {
tx.GetCollection(VARIABLE_REQBODY_PROCESSOR).Add("", "MULTIPART")
tx.GetCollection(VARIABLE_REQBODY_PROCESSOR).Set("", []string{"MULTIPART"})
}
} else if key == "host" {
tx.GetCollection(VARIABLE_SERVER_NAME).Add("", value)
tx.GetCollection(VARIABLE_SERVER_NAME).Set("", []string{value})
} else if key == "cookie" {
// Cookies use the same syntax as GET params but with semicolon (;) separator
values := utils.ParseQuery(value, ";")
Expand Down Expand Up @@ -253,7 +253,7 @@ func (tx *Transaction) SetFullRequest() {
tx.GetCollection(VARIABLE_REQUEST_LINE).GetFirstString(""),
headers,
tx.GetCollection(VARIABLE_REQUEST_BODY).GetFirstString(""))
tx.GetCollection(VARIABLE_FULL_REQUEST).Add("", full_request)
tx.GetCollection(VARIABLE_FULL_REQUEST).Set("", []string{full_request})
}

// AddResponseHeader Adds a response header variable
Expand All @@ -270,7 +270,7 @@ func (tx *Transaction) AddResponseHeader(key string, value string) {
//Most headers can be managed like that
if key == "content-type" {
spl := strings.SplitN(value, ";", 2)
tx.GetCollection(VARIABLE_RESPONSE_CONTENT_TYPE).Add("", spl[0])
tx.GetCollection(VARIABLE_RESPONSE_CONTENT_TYPE).Set("", []string{spl[0]})
}
}

Expand Down Expand Up @@ -558,11 +558,11 @@ func (tx *Transaction) ProcessConnection(client string, cPort int, server string
// tx.GetCollection(VARIABLE_REMOTE_HOST).Set("", []string{client})
// }

tx.GetCollection(VARIABLE_REMOTE_ADDR).Add("", client)
tx.GetCollection(VARIABLE_REMOTE_PORT).Add("", p)
tx.GetCollection(VARIABLE_SERVER_ADDR).Add("", server)
tx.GetCollection(VARIABLE_SERVER_PORT).Add("", p2)
tx.GetCollection(VARIABLE_UNIQUE_ID).Add("", tx.Id)
tx.GetCollection(VARIABLE_REMOTE_ADDR).Set("", []string{client})
tx.GetCollection(VARIABLE_REMOTE_PORT).Set("", []string{p})
tx.GetCollection(VARIABLE_SERVER_ADDR).Set("", []string{server})
tx.GetCollection(VARIABLE_SERVER_PORT).Set("", []string{p2})
tx.GetCollection(VARIABLE_UNIQUE_ID).Set("", []string{tx.Id})
}

// ExtractArguments transforms an url encoded string to a map and creates
Expand Down Expand Up @@ -613,12 +613,12 @@ func (tx *Transaction) AddArgument(orig string, key string, value string) {
// SecLanguage phase 1 and 2.
// note: This function won't add GET arguments, they must be added with AddArgument
func (tx *Transaction) ProcessUri(uri string, method string, httpVersion string) {
tx.GetCollection(VARIABLE_REQUEST_METHOD).Add("", method)
tx.GetCollection(VARIABLE_REQUEST_PROTOCOL).Add("", httpVersion)
tx.GetCollection(VARIABLE_REQUEST_URI_RAW).Add("", uri)
tx.GetCollection(VARIABLE_REQUEST_METHOD).Set("", []string{method})
tx.GetCollection(VARIABLE_REQUEST_PROTOCOL).Set("", []string{httpVersion})
tx.GetCollection(VARIABLE_REQUEST_URI_RAW).Set("", []string{uri})

//TODO modsecurity uses HTTP/${VERSION} instead of just version, let's check it out
tx.GetCollection(VARIABLE_REQUEST_LINE).Add("", fmt.Sprintf("%s %s %s", method, uri, httpVersion))
tx.GetCollection(VARIABLE_REQUEST_LINE).Set("", []string{fmt.Sprintf("%s %s %s", method, uri, httpVersion)})

var err error

Expand All @@ -640,22 +640,22 @@ func (tx *Transaction) ProcessUri(uri string, method string, httpVersion string)
} else {
path = uri
}
tx.GetCollection(VARIABLE_REQUEST_URI).Add("", uri)
tx.GetCollection(VARIABLE_REQUEST_URI).Set("", []string{uri})
} else {
tx.ExtractArguments("GET", parsedUrl.RawQuery)
tx.GetCollection(VARIABLE_REQUEST_URI).Add("", parsedUrl.String())
tx.GetCollection(VARIABLE_REQUEST_URI).Set("", []string{parsedUrl.String()})
path = parsedUrl.Path
query = parsedUrl.RawQuery
}
offset := strings.LastIndexAny(path, "/\\")
if offset != -1 && len(path) > offset+1 {
tx.GetCollection(VARIABLE_REQUEST_BASENAME).Add("", path[offset+1:])
tx.GetCollection(VARIABLE_REQUEST_BASENAME).Set("", []string{path[offset+1:]})
} else {
tx.GetCollection(VARIABLE_REQUEST_BASENAME).Add("", path)
tx.GetCollection(VARIABLE_REQUEST_BASENAME).Set("", []string{path})
}
tx.GetCollection(VARIABLE_REQUEST_FILENAME).Add("", path)
tx.GetCollection(VARIABLE_REQUEST_FILENAME).Set("", []string{path})

tx.GetCollection(VARIABLE_QUERY_STRING).Add("", query)
tx.GetCollection(VARIABLE_QUERY_STRING).Set("", []string{query})
}

// ProcessRequestHeaders Performs the analysis on the request readers.
Expand Down Expand Up @@ -770,7 +770,7 @@ func (tx *Transaction) ProcessRequestBody() (*Interruption, error) {
fs.Add("", fmt.Sprintf("%d", header.Size))
}
}
tx.GetCollection(VARIABLE_FILES_COMBINED_SIZE).Add("", fmt.Sprintf("%d", totalSize))
tx.GetCollection(VARIABLE_FILES_COMBINED_SIZE).Set("", []string{fmt.Sprintf("%d", totalSize)})
for k, vs := range req.MultipartForm.Value {
for _, v := range vs {
tx.AddArgument("POST", k, v)
Expand All @@ -795,8 +795,8 @@ func (tx *Transaction) ProcessRequestBody() (*Interruption, error) {
//
func (tx *Transaction) ProcessResponseHeaders(code int, proto string) *Interruption {
c := strconv.Itoa(code)
tx.GetCollection(VARIABLE_RESPONSE_STATUS).Add("", c)
tx.GetCollection(VARIABLE_RESPONSE_PROTOCOL).Add("", proto)
tx.GetCollection(VARIABLE_RESPONSE_STATUS).Set("", []string{c})
tx.GetCollection(VARIABLE_RESPONSE_PROTOCOL).Set("", []string{proto})

if tx.RuleEngine == RULE_ENGINE_OFF {
return nil
Expand Down
35 changes: 33 additions & 2 deletions waf.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,48 @@ func (w *Waf) NewTransaction() *Transaction {
tx.collections[i] = NewCollection(VariableToName(byte(i)))
}

// we must initialize single variables
for i := 0x00; i <= VARIABLE_SESSIONID; i++ {
tx.GetCollection(byte(i)).Set("", []string{""})
}

// set capture variables
txvar := tx.GetCollection(VARIABLE_TX)
for i := 0; i <= 10; i++ {
is := strconv.Itoa(i)
txvar.Set(is, []string{})
txvar.Set(is, []string{""})
}

// Some defaults
defaults := map[byte]string{
VARIABLE_URI_PARSE_ERROR: "0",
VARIABLE_URI_PARSE_ERROR: "0",
VARIABLE_FILES_COMBINED_SIZE: "0",
VARIABLE_URLENCODED_ERROR: "0",
VARIABLE_FULL_REQUEST_LENGTH: "0",
VARIABLE_MULTIPART_BOUNDARY_QUOTED: "0",
VARIABLE_MULTIPART_BOUNDARY_WHITESPACE: "0",
VARIABLE_MULTIPART_CRLF_LF_LINES: "0",
VARIABLE_MULTIPART_DATA_AFTER: "0",
VARIABLE_MULTIPART_DATA_BEFORE: "0",
VARIABLE_MULTIPART_FILE_LIMIT_EXCEEDED: "0",
VARIABLE_MULTIPART_HEADER_FOLDING: "0",
VARIABLE_MULTIPART_INVALID_HEADER_FOLDING: "0",
VARIABLE_MULTIPART_INVALID_PART: "0",
VARIABLE_MULTIPART_INVALID_QUOTING: "0",
VARIABLE_MULTIPART_LF_LINE: "0",
VARIABLE_MULTIPART_MISSING_SEMICOLON: "0",
VARIABLE_MULTIPART_STRICT_ERROR: "0",
VARIABLE_MULTIPART_UNMATCHED_BOUNDARY: "0",
VARIABLE_OUTBOUND_DATA_ERROR: "0",
VARIABLE_REQBODY_ERROR: "0",
VARIABLE_REQBODY_PROCESSOR_ERROR: "0",
VARIABLE_REQUEST_BODY_LENGTH: "0",
VARIABLE_DURATION: "0",
}
for v, data := range defaults {
tx.GetCollection(v).Set("", []string{data})
}

return tx
}

Expand Down

0 comments on commit fc63d67

Please sign in to comment.