Skip to content

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

Open
Sim4n6 wants to merge 41 commits into
github:mainfrom
Sim4n6:golang-UBV
Open

Sim4n6 wants to merge 41 commits into
github:mainfrom
Sim4n6:golang-UBV

Conversation

@Sim4n6

@Sim4n6 Sim4n6 commented May 2, 2023

Copy link
Copy Markdown
Contributor

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.

Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/src/experimental/CWE-176/UnicodeBypassValidation.qhelp Outdated
Comment thread go/ql/src/experimental/CWE-176/UnicodeBypassValidation.qhelp Outdated
@github-actions

Copy link
Copy Markdown
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

Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationCustomizations.qll Outdated
@Sim4n6

Sim4n6 commented May 26, 2023

Copy link
Copy Markdown
Contributor Author

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
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/src/change-notes/2013-05-02-post-unicode-normalization-query.md Outdated
Comment thread go/ql/src/experimental/CWE-176/UnicodeBypassValidation.qhelp Outdated

@smowton smowton left a comment

Copy link
Copy Markdown
Contributor

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
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll
Comment thread go/ql/test/experimental/CWE-176/example_bad.go Outdated
@Sim4n6

Sim4n6 commented Jun 26, 2023

Copy link
Copy Markdown
Contributor Author

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

@smowton smowton left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread go/ql/lib/semmle/go/security/UnicodeBypassValidationQuery.qll Outdated
@Sim4n6

Sim4n6 commented Jun 28, 2023

Copy link
Copy Markdown
Contributor Author

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants