Skip to content

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

Merged
glaslos merged 1 commit into
glaslos:mainfrom
edznux-dd:main
May 18, 2026
Merged

fix: prevent index out-of-range panic in hasCommonSubstring#38
glaslos merged 1 commit into
glaslos:mainfrom
edznux-dd:main

Conversation

@edznux-dd

Copy link
Copy Markdown
Contributor

👋, 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

glaslos commented Apr 28, 2026

Copy link
Copy Markdown
Owner

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

@edznux-dd

Copy link
Copy Markdown
Contributor Author

@glaslos sorry, missed the notification! I just rebased, let me know :)

@glaslos

glaslos commented May 18, 2026

Copy link
Copy Markdown
Owner

Awesome, thanks a bunch!

@glaslos glaslos merged commit 210fe14 into glaslos:main May 18, 2026
1 check passed
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