Concurrency Problems using unbuffered channel

Question

I created a tool in Go that could bruteforce subdomains using Go concurrency. The problem is that it just shows first few results. I mean if the threads i specify are 10, it shows 10, if 100 then it shows 100. Any solutions for this. I am following this example.

func CheckWildcardSubdomain(state *State, domain string, words <-chan string, wg *sync.WaitGroup) {
    defer wg.Done()

    for {
        preparedSubdomain := <-words + "." + domain
        ipAddress, err := net.LookupHost(preparedSubdomain)

        if err == nil {
            if !state.WildcardIPs.ContainsAny(ipAddress) {
                if state.Verbose == true {
                    fmt.Printf("\n%s", preparedSubdomain)
                }

                state.FinalResults = append(state.FinalResults, preparedSubdomain)
            }
        }
    }
}

func RemoveWildcardSubdomains(state *State, subdomains []string) []string {
    var wg sync.WaitGroup
    var channel = make(chan string)

    wg.Add(state.Threads)

    for i := 0; i < state.Threads; i++ {
        go CheckWildcardSubdomain(state, state.Domain, channel, &wg)
    }

    for _, entry := range subdomains {
        sub := strings.Join(strings.Split(entry, ".")[:2][:], ".")
        channel <- sub
    }

    close(channel)
    wg.Wait()

    return state.FinalResults
}

Thanks in advance for your help.

score 4 · Accepted Answer · answered Apr 17 '18 at 10:03

2 mistakes that immediately stand out.

First, in CheckWildcardSubdomain() you should range over the words channel like this:

for word := range words {
    preparedSubdomain := word + "." + domain
    // ...
}

The for ... range over a channel will terminate once all values sent on the channel (sent before the channel was closed) are received. Note that the simple receive operator will not terminate nor panic if the channel is closed, instead it will yield the zero value of the channel's element type. So your original loop would never terminate. Spec: Receive operator:

A receive operation on a closed channel can always proceed immediately, yielding the element type's zero value after any previously sent values have been received.

Second, inside CheckWildcardSubdomain() the state.FinalResults field is read / modified concurrently, without synchronization. This is undefined behavior.

You must synchronize access to this field, e.g. using a mutex, or you should find other ways to communicate and collect results, e.g. using a channel.

See this related question for an elegant, efficient and scalabe way to do it:

Is this an idiomatic worker thread pool in Go?

Concurrency Problems using unbuffered channel

1 Answers1