Skip to content

fix: prevent index out-of-range panic in hasCommonSubstring#38

Open
edznux-dd wants to merge 1 commit intoglaslos:mainfrom
edznux-dd:main
Open

fix: prevent index out-of-range panic in hasCommonSubstring#38
edznux-dd wants to merge 1 commit intoglaslos:mainfrom
edznux-dd:main

Conversation

@edznux-dd
Copy link
Copy Markdown

👋, I was looking a some of my app and discovered a small bugs in the library.
So here's a fix and some description of the issue!

I took the liberty to add a fuzz test as I don't think you have any fuzz test in here yet. I've run it for a bit after the fix and didn't seem to find any other bugs :)

Description

Distance parses hash strings by splitting on : but imposes no length cap on the body segments. hasCommonSubstring allocates a hashes slice of fixed size spamSumLength - (rollingWindow - 1) = 64 - 6 = 58, then writes into it at index i-(rollingWindow-1) while looping for i = rollingWindow-1; i < s1Len; i++. When the hash body is 65 or more characters long the index reaches 58, which is out of bounds, and the runtime raises a fatal panic.

Crash Input

I've added a regression test for this case, but for convenience:

s1 := "3:" + strings.Repeat("0", 65) + ":"
s2 := "3:0000000:"
ssdeep.Distance(s1, s2) // panic: runtime error: index out of range [58] with length 58

Root Cause

In score.go:104:

// Vulnerable:
func hasCommonSubstring(s1, s2 string) bool {
    // ...
    hashes := make([]uint32, (spamSumLength - (rollingWindow - 1))) // length = 58
    // ...
    for i = rollingWindow - 1; i < s1Len; i++ {
        state.rollHash(s1[i])
        hashes[i-(rollingWindow-1)] = state.rollSum() // panics when i-(rollingWindow-1) >= 58
    }
    // ...
}

@glaslos
Copy link
Copy Markdown
Owner

glaslos commented Apr 28, 2026

@edznux-dd can you rebase? I updated main to go1.18 to support testing.F

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants