Skip to content

Commit

Permalink
phpcs:chore - Update PHP_CodeSniffer to show severity and code (#935)
Browse files Browse the repository at this point in the history
I update the PHP_CodeSniffer tool to have a little
more information such as:
- All warnings can be shown as long as their severity is above 5
- All vulnerabilities pointed out have a correct level
of severity based on the tool
- We now demonstrate the vulnerable code in the analysis output

Signed-off-by: wilian <wilian.silva@zup.com.br>
(cherry picked from commit 5d8b435)
  • Loading branch information
wiliansilvazup authored and matheusalcantarazup committed Jan 21, 2022
1 parent 38dfc86 commit 077d21b
Show file tree
Hide file tree
Showing 8 changed files with 1,299 additions and 30 deletions.
6 changes: 3 additions & 3 deletions e2e/analysis/test_case.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ func NewTestCase() []*TestCase {
fmt.Sprintf(messages.MsgPrintFinishAnalysisWithStatus, analysis.Success),
messages.MsgDebugVulnHashToFix,
messages.MsgWarnAnalysisFoundVulns[16:],
"In this analysis, a total of 66 possible vulnerabilities were found and we classified them into:",
"Total of Vulnerability CRITICAL is: 17",
"Total of Vulnerability HIGH is: 22",
"In this analysis, a total of 69 possible vulnerabilities were found and we classified them into:",
"Total of Vulnerability CRITICAL is: 18",
"Total of Vulnerability HIGH is: 24",
"Total of Vulnerability MEDIUM is: 24",
"Total of Vulnerability LOW is: 3",
fmt.Sprintf("{HORUSEC_CLI} Running %s - %s", tools.HorusecEngine, languages.CSharp),
Expand Down
2 changes: 1 addition & 1 deletion examples
Submodule examples updated 34 files
+1 −1 Makefile
+43 −20 README.md
+1 −1 elixir/example1/mix.exs
+2 −2 elixir/example1/mix.lock
+0 −21 horusec-config.json
+0 −44 java/example1/.classpath
+27 −0 java/example1/.gitignore
+0 −23 java/example1/.project
+21 −0 java/example1/LICENSE.txt
+19 −0 java/example1/build.gradle
+5 −0 java/example1/gradle/wrapper/gradle-wrapper.properties
+185 −0 java/example1/gradlew
+89 −0 java/example1/gradlew.bat
+0 −53 java/example1/pom.xml
+2 −0 java/example1/settings.gradle
+8 −13 java/example1/src/main/java/br/com/zup/vulnerabilities/random/RandomIssue.java
+27 −0 java/example1/src/main/java/br/com/zup/vulnerabilities/random/RandomSecure.java
+13 −31 java/example1/src/main/java/br/com/zup/vulnerabilities/trust/AllTrustManagerIssue.java
+78 −0 java/example1/src/main/java/br/com/zup/vulnerabilities/trust/AllTrustSSLSocketFactoryIssue.java
+167 −0 java/example2/.gitignore
+16 −0 java/example2/build.gradle
+5 −0 java/example2/gradle/wrapper/gradle-wrapper.properties
+185 −0 java/example2/gradlew
+89 −0 java/example2/gradlew.bat
+2 −0 java/example2/settings.gradle
+37 −0 java/example2/src/main/java/br/com/zup/vulnerabilities/MyApp.java
+45 −0 java/example2/src/main/resources/log4j2.xml
+30 −0 php/example2/basic-collection.php
+24 −0 php/example2/cross-site-scripting-xss.php
+31 −0 php/example2/sql-injection.php
+30 −0 php/example2/sql-injection_2.php
+23 −0 php/example2/tool-examples/php-security-scanner.php
+127 −0 php/example2/tool-examples/phpcs-security-audit.php
+19 −0 php/example2/tool-examples/progpilot.php
27 changes: 26 additions & 1 deletion internal/controllers/language_detect/language_detect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ func TestLanguageDetect(t *testing.T) {
assert.Contains(t, langs, languages.Leaks)
assert.Contains(t, langs, languages.Java)
assert.Contains(t, langs, languages.Generic)
assert.Len(t, langs, 3)
assert.Contains(t, langs, languages.Shell)
assert.Len(t, langs, 4)
})

t.Run("Should run language detect and return JAVASCRIPT and GITLEAKS", func(t *testing.T) {
Expand Down Expand Up @@ -212,6 +213,30 @@ func TestLanguageDetect(t *testing.T) {
assert.Len(t, langs, 4)
})

t.Run("Should run language detect and return PHP, LEAKS, GENERIC in example2", func(t *testing.T) {
controller := NewLanguageDetect(config.New(), uuid.New())

langs, err := controller.Detect(testutil.PHPExample2)

assert.NoError(t, err)
assert.Contains(t, langs, languages.Leaks)
assert.Contains(t, langs, languages.Generic)
assert.Contains(t, langs, languages.PHP)
assert.Len(t, langs, 3)
})

t.Run("Should run language detect and return PHP, LEAKS, GENERIC in example1", func(t *testing.T) {
controller := NewLanguageDetect(config.New(), uuid.New())

langs, err := controller.Detect(testutil.PHPExample1)

assert.NoError(t, err)
assert.Contains(t, langs, languages.Leaks)
assert.Contains(t, langs, languages.Generic)
assert.Contains(t, langs, languages.PHP)
assert.Len(t, langs, 3)
})

t.Run("Should run language detect and return KOTLIN and GITLEAKS", func(t *testing.T) {
controller := NewLanguageDetect(config.New(), uuid.New())

Expand Down
60 changes: 53 additions & 7 deletions internal/services/formatters/php/phpcs/entities/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,29 @@ package entities
import (
"strconv"
"strings"

"github.com/ZupIT/horusec-devkit/pkg/enums/severities"
)

const (
MessageError = "ERROR"
MessageWarning = "WARNING"
)

const (
SeverityLevelLow = 2
SeverityLevelMedium = 3
SeverityLevelHigh = 4
SeverityLevelCritical = 5
)

type Message struct {
Message string `json:"message"`
Line int `json:"line"`
Column int `json:"column"`
Type string `json:"type"`
Severity int `json:"severity"`
Source string `json:"source"`
Message string `json:"message"`
Line int `json:"line"`
Column int `json:"column"`
Type string `json:"type"`
}

func (m *Message) GetLine() string {
Expand All @@ -34,7 +50,37 @@ func (m *Message) GetColumn() string {
return strconv.Itoa(m.Column)
}

func (m *Message) IsValidMessage() bool {
return m.Type == "ERROR" &&
!strings.Contains(m.Message, "This implies that some PHP code is not scanned by PHPCS")
func (m *Message) IsAllowedMessage() bool {
return m.isWarningMessage() || m.isErrorMessage()
}

func (m *Message) isErrorMessage() bool {
return m.Type == MessageError
}

func (m *Message) isWarningMessage() bool {
return m.Type == MessageWarning
}

func (m *Message) IsNotFailedToScan() bool {
return !strings.Contains(strings.ToLower(m.Message),
"this implies that some php code is not scanned by phpcs")
}

// nolint:funlen,gocyclo // method of validation
func (m *Message) GetSeverity() severities.Severity {
switch {
case m.isWarningMessage() && m.Severity >= SeverityLevelCritical:
return severities.Info
case m.Severity < SeverityLevelLow:
return severities.Low
case m.Severity >= SeverityLevelLow && m.Severity <= SeverityLevelMedium:
return severities.Medium
case m.Severity == SeverityLevelHigh:
return severities.High
case m.Severity >= SeverityLevelCritical:
return severities.Critical
default:
return severities.Unknown
}
}
25 changes: 17 additions & 8 deletions internal/services/formatters/php/phpcs/entities/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,30 @@ func TestGetColumn(t *testing.T) {
}

func TestIsValidMessage(t *testing.T) {
t.Run("should return false if invalid message", func(t *testing.T) {
t.Run("should return true if valid error message", func(t *testing.T) {
message := &Message{
Message: "This implies that some PHP code is not scanned by PHPCS",
Type: "ERROR",
Type: "ERROR",
}

assert.False(t, message.IsValidMessage())
assert.True(t, message.isErrorMessage())
assert.True(t, message.IsAllowedMessage())
})

t.Run("should return true if valid message", func(t *testing.T) {
t.Run("should return true if valid warning message", func(t *testing.T) {
message := &Message{
Message: "",
Type: "ERROR",
Type: "WARNING",
}

assert.True(t, message.IsValidMessage())
assert.True(t, message.isWarningMessage())
assert.True(t, message.IsAllowedMessage())
})
t.Run("should return false if invalid type message", func(t *testing.T) {
message := &Message{
Type: "other",
}

assert.False(t, message.isErrorMessage())
assert.False(t, message.isWarningMessage())
assert.False(t, message.IsAllowedMessage())
})
}
15 changes: 11 additions & 4 deletions internal/services/formatters/php/phpcs/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ package phpcs

import (
"encoding/json"
"fmt"

"github.com/ZupIT/horusec-devkit/pkg/entities/vulnerability"
"github.com/ZupIT/horusec-devkit/pkg/enums/languages"
"github.com/ZupIT/horusec-devkit/pkg/enums/severities"
"github.com/ZupIT/horusec-devkit/pkg/enums/tools"
"github.com/ZupIT/horusec-devkit/pkg/utils/logger"

Expand All @@ -28,6 +28,7 @@ import (
"github.com/ZupIT/horusec/internal/helpers/messages"
"github.com/ZupIT/horusec/internal/services/formatters"
"github.com/ZupIT/horusec/internal/services/formatters/php/phpcs/entities"
fileutils "github.com/ZupIT/horusec/internal/utils/file"
vulnhash "github.com/ZupIT/horusec/internal/utils/vuln_hash"
)

Expand Down Expand Up @@ -94,20 +95,26 @@ func (f *Formatter) parseResults(results map[string]interface{}) {

func (f *Formatter) parseMessages(filepath string, result interface{}) {
for _, message := range f.parseToResult(result).Messages {
if message.IsValidMessage() {
f.AddNewVulnerabilityIntoAnalysis(f.setVulnerabilityData(filepath, message))
if message.IsAllowedMessage() && message.IsNotFailedToScan() {
vuln := f.setVulnerabilityData(filepath, message)
f.AddNewVulnerabilityIntoAnalysis(vuln)
}
}
}

func (f *Formatter) setVulnerabilityData(filepath string, result entities.Message) *vulnerability.Vulnerability {
vuln := f.getDefaultVulnerabilitySeverity()
vuln.Severity = severities.Unknown
vuln.Severity = result.GetSeverity()
vuln.Details = result.Message
vuln.Line = result.GetLine()
vuln.Column = result.GetColumn()
vuln.File = f.RemoveSrcFolderFromPath(filepath)
vuln = vulnhash.Bind(vuln)

// NOTE: code and details were set after the vulnhash.Bind to avoid changes in the hash generation
vuln.Code, _ = fileutils.GetCode(f.GetConfigProjectPath(), vuln.File, result.GetLine())
vuln.Details += fmt.Sprintf("\n %s", result.Source)

return f.SetCommitAuthor(vuln)
}

Expand Down
Loading

0 comments on commit 077d21b

Please sign in to comment.