Skip to content

[Go] Add Unicode Bypass Validation query, test and help file #12994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

Sim4n6
Copy link
Contributor

@Sim4n6 Sim4n6 commented May 2, 2023

This pull request adds a Unicode Bypass Validation (UBV) query, tests, and the help file.

The UBV query checks for a Post-Unicode Normalization in a Golang codebase that leads to some security issues such as the bypasses of a String validation, regex verification, and escape functions.

@github-actions
Copy link
Contributor

QHelp previews:

go/ql/src/experimental/CWE-176/UnicodeBypassValidation.qhelp

Bypass Logical Validation Using Unicode Characters

Security checks bypass due to a Unicode transformation

If ever a unicode tranformation is performed after some security checks or logical validation, the latter could be bypassed due to a potential Unicode characters collision. The validation of concern are any character escaping, any regex validation or any string verification.

Security checks bypassed

Recommendation

Perform a Unicode normalization before the logical validation.

Example

The following example showcases the bypass of all checks performed by html.EscapeString() due to a post-unicode normalization.

For instance: the character U+FE64 () is not filtered-out by the flask escape function. But due to the Unicode normalization, the character is transformed and would become U+003C (< ).

package main

import (
	"fmt"
	"html"
	"net/http"

	"golang.org/x/text/unicode/norm"
)

func main() {}

func bad() {
	http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {

		unicode_input := req.URL.Query().Get("unicode_input")
		escaped := html.EscapeString(unicode_input)
		unicode_norm := norm.NFKC.String(escaped)
		fmt.Println(w, "Results: %q", unicode_norm)
	})
}

References

@Sim4n6
Copy link
Contributor Author

Sim4n6 commented May 26, 2023

Sorry about the delay, I will be working on this query this weekend.

@Sim4n6 Sim4n6 requested a review from smowton May 27, 2023 17:27
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, couple more adjustments

@Sim4n6 Sim4n6 requested a review from smowton May 30, 2023 18:43
@Sim4n6 Sim4n6 requested a review from mbg June 6, 2023 16:09
@Sim4n6
Copy link
Contributor Author

Sim4n6 commented Jun 26, 2023

A regex match function calls could be used as a barrier guard, commit a64a998.

@Sim4n6 Sim4n6 requested a review from smowton June 26, 2023 10:33
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good now! I note the bounty application wants you to submit a Golang CVE -- once that's done I think securitylab will now proceed to check his query's accuracy.

@Sim4n6
Copy link
Contributor Author

Sim4n6 commented Jun 28, 2023

Thanks, no problem @smowton

@mbg mbg removed their request for review November 24, 2023 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Go no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants