ooni / probe

OONI Probe network measurement tool for detecting internet censorship
https://ooni.org/install
BSD 3-Clause "New" or "Revised" License
749 stars 142 forks source link

engine: never unconditionally cast to richer input target #2753

Closed bassosimone closed 1 week ago

bassosimone commented 1 week ago

The following code, extracted from https://github.com/ooni/probe-cli/blob/5be3a9a2c7a4ec9825054e4669c5725037e22578/internal/experiment/dnscheck/dnscheck.go#L132:

// Run implements model.ExperimentSession.Run
func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
    _ = args.Callbacks
    measurement := args.Measurement
    sess := args.Session

    // 0. obtain the richer input target, config, and input or panic
    if args.Target == nil {
        return ErrInputRequired
    }
    target := args.Target.(*Target) // <- WRONG

is wrong where indicated. While it is true that the cast will always succeed within the Go codebase, we need to introduce more robustness thinking of the case where the mobile app delivers richer input to the probe. In such a case, it might well happen that the provided type implements the interface but it's not using the correct type. This is why it is crucial for us to handle this case gracefully rather than crashing.

There may be other alike cases and to be sure we need to review all the richer input work merged so far and make sure that we not only fix dnscheck but all the other alike cases.