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.

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

Sending 2000, 4000 or 6000 characters to Discord panics in util.PartitionMessage (index out of range) #240

Closed
justinsteven opened this issue May 21, 2022 · 0 comments · Fixed by #242

Comments

@justinsteven
Copy link
Contributor

What happens

Sending exactly 2000, 4000 or 6000 characters to Discord panics

What should happen

Sending any number of characters to Discord should succeed

Demo

package main

import (
	"fmt"
	t "github.com/containrrr/shoutrrr/pkg/types"
	"strings"
)

// BEGIN taken from shoutrrr 0.5.3 util and discord packages

var limits = t.MessageLimit{
	ChunkSize:      2000,
	TotalChunkSize: 6000,
	ChunkCount:     10,
}

const maxSearchRunes = 100

// Min returns the smallest of a and b
func Min(a int, b int) int {
	if a < b {
		return a
	}
	return b
}

// PartitionMessage splits a string into chunks that is at most chunkSize runes, it will search the last distance runes
// for a whitespace to make the split appear nicer. It will keep adding chunks until it reaches maxCount chunks, or if
// the total amount of runes in the chunks reach maxTotal.
// The chunks are returned together with the number of omitted runes (that did not fit into the chunks)
func PartitionMessage(input string, limits t.MessageLimit, distance int) (items []t.MessageItem, omitted int) {
	runes := []rune(input)
	chunkOffset := 0
	maxTotal := Min(len(runes), limits.TotalChunkSize)
	maxCount := limits.ChunkCount - 1

	for i := 0; i < maxCount; i++ {
		// If no suitable split point is found, use the chunkSize
		chunkEnd := chunkOffset + limits.ChunkSize
		// ... and start next chunk directly after this one
		nextChunkStart := chunkEnd
		if chunkEnd > maxTotal {
			// The chunk is smaller than the limit, no need to search
			chunkEnd = maxTotal
			nextChunkStart = maxTotal
		} else {
			for r := 0; r < distance; r++ {
				rp := chunkEnd - r
				if runes[rp] == '\n' || runes[rp] == ' ' {
					// Suitable split point found
					chunkEnd = rp
					// Since the split is on a whitespace, skip it in the next chunk
					nextChunkStart = chunkEnd + 1
					break
				}
			}
		}

		items = append(items, t.MessageItem{
			Text: string(runes[chunkOffset:chunkEnd]),
		})

		chunkOffset = nextChunkStart
		if chunkOffset >= maxTotal {
			break
		}
	}

	return items, len(runes) - chunkOffset
}

// END taken from shoutrrr 0.5.3

func fuzz(length int) {
	defer func() {
		if r := recover(); r != nil {
			fmt.Printf("Recovered panic when partitioning message of length %d: %s\n", length, r)
		}
	}()
	PartitionMessage(strings.Repeat("A", length), limits, maxSearchRunes)
}

func main() {
	for i := 0; i <= 20000; i++ {
		fuzz(i)
	}
}
% go run shoutrrr_partitionmessage_fuzz.go
Recovered panic when partitioning message of length 2000: runtime error: index out of range [2000] with length 2000
Recovered panic when partitioning message of length 4000: runtime error: index out of range [4000] with length 4000
Recovered panic when partitioning message of length 6000: runtime error: index out of range [6000] with length 6000

Notes

There is also a report of util.PartitionMessage panicking with what seems to be a message of 3990 characters - see projectdiscovery/notify#130 (review). I haven't been able to reproduce this crash. It may or may not be related.

This is a potential DoS vulnerability. If an attacker can cause a consumer of shoutrrr to attempt to send a Discord message of a precise length, the consumer will panic, rendering the service unavailable. Without a published security policy I don't have a way of discretely reporting this. May I suggest you publish a policy :)

justinsteven added a commit to justinsteven/notify that referenced this issue May 22, 2022
Inclues fixes for:

* containrrr/shoutrrr#238
  * Sending more than ~99 lines to Slack fails with too_many_attachments
* containrrr/shoutrrr#240
  * Sending 2000, 4000 or 6000 characters to Discord panics
* containrrr/shoutrrr#244
  * Sending 1999, 3999 or 5999 characters to Discord panics
  * (Incomplete fix for the above)
ehsandeep added a commit to projectdiscovery/notify that referenced this issue May 30, 2022
* go.mod update to 1.17 and dependency synchronisation

* Docker file update to use go 1.18 and "go install" instead of "go get"

* Minor documentation updates

* GitHub action version updates

* goreleaser-action version fix

* Allow -bulk to work with stdin

* go mod update

* replaced helper function with lib

* Do chunked reading in -bulk mode

Reduces memory usage when input is big

Also includes the fix for #134 (makes testing easier)

* Move bulkSplitter to util.go

* Remove unused function

* Refactor bulkSplitter

Simplified looping. Also, we no longer truncate mid-line unless it's the first
line of a chunk (in such a case we have no option but to truncate). A truncated
line will be followed by an ellipsis. Otherwise we're splitting chunks at the
last possible newline without exceeding charLimit for that chunk.

* Bugfix: Always remove trailing newlines

* Bump shoutrrr for long message fixes

Inclues fixes for:

* containrrr/shoutrrr#238
  * Sending more than ~99 lines to Slack fails with too_many_attachments
* containrrr/shoutrrr#240
  * Sending 2000, 4000 or 6000 characters to Discord panics
* containrrr/shoutrrr#244
  * Sending 1999, 3999 or 5999 characters to Discord panics
  * (Incomplete fix for the above)

* minor changes

Co-authored-by: Sandeep Singh <sandeep@projectdiscovery.io>
Co-authored-by: forgedhallpass <13679401+forgedhallpass@users.noreply.github.com>
Co-authored-by: mzack <marco.rivoli.nvh@gmail.com>
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 a pull request may close this issue.

1 participant