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

fix: attr with multiple nulls #30

Merged
merged 1 commit into from
Jun 10, 2024
Merged

fix: attr with multiple nulls #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions xss_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ func isBlackTag(s string) bool {
return false
}

upperS := strings.ToUpper(strings.ReplaceAll(s, "\x00", ""))
sUpperWithoutNulls := strings.ToUpper(strings.ReplaceAll(s, "\x00", ""))
for i := 0; i < len(blackTags); i++ {
if upperS == blackTags[i] {
if sUpperWithoutNulls == blackTags[i] {
return true
}
}

switch upperS {
switch sUpperWithoutNulls {
// anything SVG or XSL(t) related
case "SVT", "XSL":
return true
Expand All @@ -30,12 +30,12 @@ func isBlackTag(s string) bool {
}

func isBlackAttr(s string) int {
length := len(s)
sUpperWithoutNulls := strings.ToUpper(strings.ReplaceAll(s, "\x00", ""))

length := len(sUpperWithoutNulls)
Copy link
Member Author

Choose a reason for hiding this comment

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

length has to be calculated after stripping off the null bytes. Otherwise sUpperWithoutNulls[:2] might then fail as happened in the fuzz test: panic: runtime error: slice bounds out of range [:2] with length 1

if length < 2 {
return attributeTypeNone
}

sUpperWithoutNulls := strings.ToUpper(strings.ReplaceAll(s, "\x00", ""))
if length >= 5 {
if sUpperWithoutNulls == "XMLNS" || sUpperWithoutNulls == "XLINK" {
// got xmlns or xlink tags
Expand Down
47 changes: 47 additions & 0 deletions xss_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package libinjection

import (
"testing"
)

func TestIsBlackAttr(t *testing.T) {
tests := []struct {
name string
attr string
want int
}{
{
name: "Test with black attribute",
attr: "xmlns",
want: attributeTypeBlack,
},
{
name: "Test with non-black attribute",
attr: "class",
want: attributeTypeNone,
},
{
name: "Test with JavaScript event handler",
attr: "onclick",
want: attributeTypeBlack,
},
{
name: "Test with short attribute",
attr: "a",
want: attributeTypeNone,
},
{
name: "Test with long null attribute that will be stripped",
attr: "a\x00\x00\x00\x00\x00",
want: attributeTypeNone,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := isBlackAttr(tt.attr); got != tt.want {
t.Errorf("isBlackAttr() = %v, want %v", got, tt.want)
}
})
}
}
Loading