Skip to content

Commit

Permalink
Test against known security advisories
Browse files Browse the repository at this point in the history 
Create a new test suite that tests ades against known security
advisories to ensure they're being detected as expected.

Part of this uncovered a bug in the `-conservative` implementation
which would miss expressions if they contain more than just a known
problematic expression (see the updated tests for examples).

Signed-off-by: Eric Cornelissen <ericornelissen@gmail.com>
  • Loading branch information
@ericcornelissen
ericcornelissen committed Dec 14, 2024
1 parent e77fef7 commit 3e647e8
Show file tree
Hide file tree
Showing 4 changed files with 865 additions and 29 deletions.
2 changes: 1 addition & 1 deletion matchers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (m allExprMatcher) FindAll(v []byte) [][]byte {
return allExprRegExp.FindAll(v, len(v))
}

var conservativeExprRegExp = regexp.MustCompile(`\$\{\{\s*(github\.event\.issue\.title|github\.event\.issue\.body|github\.event\.discussion\.title|github\.event\.discussion\.body|github\.event\.comment\.body|github\.event\.review\.body|github\.event\.review_comment\.body|github\.event\.pages\[\d+\]\.page_name|github\.event\.commits\[\d+\]\.message|github\.event\.commits\[\d+\]\.author\.email|github\.event\.commits\[\d+\]\.author\.name|github\.event\.head_commit\.message|github\.event\.head_commit\.author\.email|github\.event\.head_commit\.author\.name|github\.event\.head_commit\.committer\.email|github\.event\.workflow_run\.head_branch|github\.event\.workflow_run\.head_commit\.message|github\.event\.workflow_run\.head_commit\.author\.email|github\.event\.workflow_run\.head_commit\.author\.name|github\.event\.pull_request\.title|github\.event\.pull_request\.body|github\.event\.pull_request\.head\.label|github\.event\.pull_request\.head\.repo\.default_branch|github\.head_ref|github\.event\.pull_request\.head\.ref|github\.event\.workflow_run\.pull_requests\[\d+\]\.head\.ref)\s*\}\}`)
var conservativeExprRegExp = regexp.MustCompile(`\$\{\{.+?(github\.event\.issue\.title|github\.event\.issue\.body|github\.event\.discussion\.title|github\.event\.discussion\.body|github\.event\.comment\.body|github\.event\.review\.body|github\.event\.review_comment\.body|github\.event\.pages\[\d+\]\.page_name|github\.event\.commits\[\d+\]\.message|github\.event\.commits\[\d+\]\.author\.email|github\.event\.commits\[\d+\]\.author\.name|github\.event\.head_commit\.message|github\.event\.head_commit\.author\.email|github\.event\.head_commit\.author\.name|github\.event\.head_commit\.committer\.email|github\.event\.workflow_run\.head_branch|github\.event\.workflow_run\.head_commit\.message|github\.event\.workflow_run\.head_commit\.author\.email|github\.event\.workflow_run\.head_commit\.author\.name|github\.event\.pull_request\.title|github\.event\.pull_request\.body|github\.event\.pull_request\.head\.label|github\.event\.pull_request\.head\.repo\.default_branch|github\.head_ref|github\.event\.pull_request\.head\.ref|github\.event\.workflow_run\.pull_requests\[\d+\]\.head\.ref).+?\}\}`)

type conservativeExprMatcher struct{}

Expand Down
68 changes: 40 additions & 28 deletions matchers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,164 +121,176 @@ func TestConservativeMatcher(t *testing.T) {
want []string
}

testCases := []TestCase{
{
testCases := map[string]TestCase{
"github.event.issue.title": {
value: "${{ github.event.issue.title }}",
want: []string{
"${{ github.event.issue.title }}",
},
},
{
"github.event.issue.body": {
value: "${{ github.event.issue.body }}",
want: []string{
"${{ github.event.issue.body }}",
},
},
{
"github.event.discussion.title": {
value: "${{ github.event.discussion.title }}",
want: []string{
"${{ github.event.discussion.title }}",
},
},
{
"github.event.discussion.body": {
value: "${{ github.event.discussion.body }}",
want: []string{
"${{ github.event.discussion.body }}",
},
},
{
"github.event.comment.body": {
value: "${{ github.event.comment.body }}",
want: []string{
"${{ github.event.comment.body }}",
},
},
{
"github.event.review.body": {
value: "${{ github.event.review.body }}",
want: []string{
"${{ github.event.review.body }}",
},
},
{
"github.event.review_comment.body": {
value: "${{ github.event.review_comment.body }}",
want: []string{
"${{ github.event.review_comment.body }}",
},
},
{
"github.event.pages[*].page_name": {
value: "${{ github.event.pages[0].page_name }}",
want: []string{
"${{ github.event.pages[0].page_name }}",
},
},
{
"github.event.commits[*].message": {
value: "${{ github.event.commits[1].message }}",
want: []string{
"${{ github.event.commits[1].message }}",
},
},
{
"github.event.commits[*].author.email": {
value: "${{ github.event.commits[2].author.email }}",
want: []string{
"${{ github.event.commits[2].author.email }}",
},
},
{
"github.event.commits[*].author.name": {
value: "${{ github.event.commits[3].author.name }}",
want: []string{
"${{ github.event.commits[3].author.name }}",
},
},
{
"github.event.head_commit.message": {
value: "${{ github.event.head_commit.message }}",
want: []string{
"${{ github.event.head_commit.message }}",
},
},
{
"github.event.head_commit.author.email": {
value: "${{ github.event.head_commit.author.email }}",
want: []string{
"${{ github.event.head_commit.author.email }}",
},
},
{
"github.event.head_commit.author.name": {
value: "${{ github.event.head_commit.author.name }}",
want: []string{
"${{ github.event.head_commit.author.name }}",
},
},
{
"github.event.head_commit.committer.email": {
value: "${{ github.event.head_commit.committer.email }}",
want: []string{
"${{ github.event.head_commit.committer.email }}",
},
},
{
"github.event.workflow_run.head_branch": {
value: "${{ github.event.workflow_run.head_branch }}",
want: []string{
"${{ github.event.workflow_run.head_branch }}",
},
},
{
"github.event.workflow_run.head_commit.message": {
value: "${{ github.event.workflow_run.head_commit.message }}",
want: []string{
"${{ github.event.workflow_run.head_commit.message }}",
},
},
{
"github.event.workflow_run.head_commit.author.email": {
value: "${{ github.event.workflow_run.head_commit.author.email }}",
want: []string{
"${{ github.event.workflow_run.head_commit.author.email }}",
},
},
{
"github.event.workflow_run.head_commit.author.name": {
value: "${{ github.event.workflow_run.head_commit.author.name }}",
want: []string{
"${{ github.event.workflow_run.head_commit.author.name }}",
},
},
{
"github.event.pull_request.title": {
value: "${{ github.event.pull_request.title }}",
want: []string{
"${{ github.event.pull_request.title }}",
},
},
{
"github.event.pull_request.body": {
value: "${{ github.event.pull_request.body }}",
want: []string{
"${{ github.event.pull_request.body }}",
},
},
{
"github.event.pull_request.head.label": {
value: "${{ github.event.pull_request.head.label }}",
want: []string{
"${{ github.event.pull_request.head.label }}",
},
},
{
"github.event.pull_request.head.repo.default_branch": {
value: "${{ github.event.pull_request.head.repo.default_branch }}",
want: []string{
"${{ github.event.pull_request.head.repo.default_branch }}",
},
},
{
"github.head_ref": {
value: "${{ github.head_ref }}",
want: []string{
"${{ github.head_ref }}",
},
},
{
"github.event.pull_request.head.ref": {
value: "${{ github.event.pull_request.head.ref }}",
want: []string{
"${{ github.event.pull_request.head.ref }}",
},
},
{
"github.event.workflow_run.pull_requests[*].head.ref": {
value: "${{ github.event.workflow_run.pull_requests[4].head.ref }}",
want: []string{
"${{ github.event.workflow_run.pull_requests[4].head.ref }}",
},
},
{
"two, both are dangerous": {
value: "${{ github.event.pull_request.head.ref || github.head_ref }}",
want: []string{
"${{ github.event.pull_request.head.ref || github.head_ref }}",
},
},
"two, only one is dangerous": {
value: "${{ github.event.pull_request.head.ref || inputs.backup }}",
want: []string{
"${{ github.event.pull_request.head.ref || inputs.backup }}",
},
},
"not conservatively dangerous": {
value: "${{ input.greeting }}",
want: []string{},
},
Expand Down
Loading

0 comments on commit 3e647e8

Please sign in to comment.