projectdiscovery / shuffledns

MassDNS wrapper written in go to enumerate valid subdomains using active bruteforce as well as resolve subdomains with wildcard filtering and easy input-output support.
https://projectdiscovery.io
GNU General Public License v3.0
1.33k stars 192 forks source link

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

Closed luke-goddard closed 2 months ago

luke-goddard commented 3 months ago

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/toolchain@v0.0.1-go1.22.4.linux-amd64/src/bufio/bufio.go:639 +0x176                                  │
bufio.(*Writer).WriteString(0xc00a682280, {0xc006087be0?, 0x0?})                                                                              │
        /home/luke/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.4.linux-amd64/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/shuffledns@v1.1.0/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/shuffledns@v1.1.0/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

Mzack9999 commented 2 months ago

Fixed in https://github.com/projectdiscovery/shuffledns/pull/376