Add more tests

[picatz]

This PR adds more tests.

[picatz]

e1653cb added a test that essentially checks if the echo function used in the handler function can be exploited to inject malicious code into the response (reflected cross-site scripting):

taint/xss/testdata/src/e/main.go

Lines 9 to 30 in e1653cb

	func echo(w io.Writer, r any) error {
	ior, ok := r.(io.Reader)
	if !ok {
	return fmt.Errorf("failed to cast to io.Reader")
	}
	b, err := io.ReadAll(ior)
	if err != nil {
	return fmt.Errorf("failed to read all bytes from io.Reader: %w", err)
	}
	w.Write(b)
	return nil
	}
	func handler(w http.ResponseWriter, r *http.Request) {
	err := echo(w, r) // want "potential XSS"
	if err != nil {
	panic(err)
	}
	}

Callgraph

n0:e.main
	→ n5:net/http.HandleFunc
	→ n7:net/http.ListenAndServe

n1:e.echo
	→ n3:(io.Writer).Write
	→ n6:io.ReadAll
	→ n8:fmt.Errorf
	→ n9:(io.Writer).Write

n3:(io.Writer).Write
	→ n4:(net/http.ResponseWriter).Write

n4:(net/http.ResponseWriter).Write

n7:net/http.ListenAndServe

n2:e.handler
	→ n1:e.echo

n5:net/http.HandleFunc
	→ n2:e.handler

n6:io.ReadAll

n8:fmt.Errorf

n9:(io.Writer).Write

Tainted Path

n5:net/http.HandleFunc → n2:e.handler → n1:e.echo → n3:(io.Writer).Write → n4:(net/http.ResponseWriter).Write

Importantly, echo function's signature obscures the test because it takes an io.Writer and empty interface r as parameters, effectively masking the type information for r.

[picatz]

🤔 Continuing to add more test cases, if we add an "f" test based e1653cb, the analysis gets confused:

e	f
func echo(w io.Writer, r any) { ior := r.(io.Reader) b, err := io.ReadAll(ior) if err != nil { panic(err) } w.Write(b) } func handler(w http.ResponseWriter, r *http.Request) { echo(w, r) // want "potential XSS" }	func echo(w io.Writer, r any) { ior := r.(io.Reader) b, err := io.ReadAll(ior) if err != nil { panic(err) } w.Write(b) } func handler(w http.ResponseWriter, r *http.Request) { b := bufio.NewWriterSize(w, 4096) defer b.Flush() echo(b, r) // want "potential XSS" }
Simplified version of e1653cb with less error handling. Current analysis logic can easily catch this.	Includes bufio.NewWriterSize, an additional call in the call graph. Current analysis logic is confused by this.

Sink Path for E

n5:net/http.HandleFunc → n2:e.handler → n1:e.echo → n3:(io.Writer).Write → n4:(net/http.ResponseWriter).Write

Sink Path for F

n7:net/http.HandleFunc → n2:f.handler → n3:bufio.NewWriterSize → n4:(io.Writer).Write → n5:(net/http.ResponseWriter).Write

This confusion likely partially stems from the "virtual" function calls now tracked for interfaces (like io.Writer) added in #9 (comment).

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[picatz]

This was a very imperfect, but fun PR to work on. Learned a lot, and I hope to follow this up in the near future.