shuffledns icon indicating copy to clipboard operation
shuffledns copied to clipboard

Race condition - Multiple concurrent calls to bufio.Writer.WriteString cause OOB crash

Open luke-goddard opened this issue 6 months ago • 0 comments

Describe the bug

Bufio.Writer is not safe when shared between goroutines without custom synchronization. Not sure how common this crash is or I was just very very unlucky. Happy to submit a PR that wraps the writer in a mutex. This issue only occurs when writing the results to a file.

Shuffledns version

grep shuffledns go.mod 
        github.com/projectdiscovery/shuffledns v1.1.0

Complete command you used to reproduce this

func RunShuffleDNSSubdomainBrute(dom string, cliOptions *DomainOptions) event.Event {
	uuid := uuid.New()
	output := "/tmp/output-" + uuid.String() + ".txt"
	var options = runner.Options{
		Domains:            []string{dom}, 
		Directory:          "/tmp",
		Output:             output,
		MassdnsPath:        "",
		Wordlist:           cliOptions.Wordlist,
		ResolversFile:      cliOptions.Resolver,
		MassDnsCmd:         "",
		StrictWildcard:     true,
		Json:               false,
		Silent:             true,
		Retries:            1,
		NoColor:            false,
		DisableUpdateCheck: true,
		Threads:            100,
		Verbose:            false,
	}

	massdnsRunner, err := runner.New(&options)
	if err != nil {
		// SNIP
	}

	massdnsRunner.RunEnumeration()
	massdnsRunner.Close()

       // SNIP for brevity
}

Screenshots

panic: runtime error: slice bounds out of range [:4120] with capacity 4096                                                                    │
                                                                                                                                              │
goroutine 2504 [running]:                                                                                                                     │
bufio.(*Writer).Flush(0x20?)                                                                                                                  │
        /home/luke/go/pkg/mod/golang.org/[email protected]/src/bufio/bufio.go:639 +0x176                                  │
bufio.(*Writer).WriteString(0xc00a682280, {0xc006087be0?, 0x0?})                                                                              │
        /home/luke/go/pkg/mod/golang.org/[email protected]/src/bufio/bufio.go:763 +0x178                                  │
github.com/projectdiscovery/shuffledns/pkg/massdns.(*Instance).writeOutput.func1.1({0xc00661b6c0, 0x1a})                                      │
        /home/luke/go/pkg/mod/github.com/projectdiscovery/[email protected]/pkg/massdns/process.go:337 +0x6c5                                 │
created by github.com/projectdiscovery/shuffledns/pkg/massdns.(*Instance).writeOutput.func1 in goroutine 70                                   │
        /home/luke/go/pkg/mod/github.com/projectdiscovery/[email protected]/pkg/massdns/process.go:303 +0x1f2                                 │
exit status 2   

We've got a writer that is shared between many go routines https://github.com/projectdiscovery/shuffledns/blob/1e45a1b42a7910fa6af00bf6071e107af73fab48/pkg/massdns/process.go#L260

    var w *bufio.Writer
	store.Iterate(func(ip string, hostnames []string, counter int) {
		for _, hostname := range hostnames {
			// SNIP
			go func(hostname string) {
			        //SNIP

				if output != nil {
					_, _ = w.WriteString(data) // Here
				}
                                // SNIP
			}(hostname)
		}
	})

If two goroutines are unlucky enough to land here at the same time b.n += n then the internal tracker of the buffer size becomes incorrect and we get the OOB error.

https://github.com/golang/go/blob/d8c7230c97ca5639389917cc235175bfe2dc50ab/src/bufio/bufio.go#L762

https://github.com/projectdiscovery/shuffledns/blob/1e45a1b42a7910fa6af00bf6071e107af73fab48/pkg/massdns/process.go#L337

luke-goddard avatar Jul 26 '24 10:07 luke-goddard