fixe(domain): wildcard parse bug

[Hvitgar]

I think there is a bug in the parseWildcardRules function - if the wildcard is at the end of the origin string, the character right before the wildcard is cut off, leading to potentially unwanted matches, for instance https://example.com/* would also validate anything on https://example.community/*.

For verification, just execute the code below:

package main

import (
	"errors"
	"strings"
)

type Config struct {
	AllowWildcard bool
	AllowOrigins  []string
}

func (c Config) parseWildcardRules() [][]string {
	var wRules [][]string

	if !c.AllowWildcard {
		return wRules
	}

	for _, o := range c.AllowOrigins {
		if !strings.Contains(o, "*") {
			continue
		}

		if c := strings.Count(o, "*"); c > 1 {
			panic(errors.New("only one * is allowed").Error())
		}

		i := strings.Index(o, "*")
		if i == 0 {
			wRules = append(wRules, []string{"*", o[1:]})
			continue
		}
		if i == (len(o) - 1) {
			wRules = append(wRules, []string{o[:i-1], "*"})
			continue
		}

		wRules = append(wRules, []string{o[:i], o[i+1:]})
	}

	return wRules
}

func main() {
	c1 := Config{AllowWildcard: true, AllowOrigins: []string{
		"https://example.com/*",
		"https://example.com/*/helloWorld",
		"*/helloWorld",
	}}

	wrules := c1.parseWildcardRules()
	for i, wr := range wrules {
		println(c1.AllowOrigins[i] + " ->")
		println(wr[0])
		println(wr[1])
		println()
	}
}

which results in

https://example.com/* ->
https://example.com                 # incorrect, should be https://example.com/
*

https://example.com/*/helloWorld ->
https://example.com/
/helloWorld

*/helloWorld ->
*
/helloWorld

[Hvitgar]

I just noticed that there is a PR open for this, however it is from 2019...so I will leave this open as a reminder

[jub0bs]

Note that Web origins do not contain a path. But I agree with you that a trailing wildcard is a terrible footgun; for instance, https://example.* would match Web origin https://example.attacker.com.

[ninogresenz]

Hey, I just discovered this bug as well. Will this be merged or what's the status?
Even though Web origins do not contain a path, it leads to the situation where you configure https://example.* and it would match https://example-without-a-dot.com. Sure it's a footgun, but it's even worse as I would consider this an unwanted behavior.

[appleboy]

I will take it.

[jub0bs]

Relevant: https://jub0bs.com/posts/2023-02-08-fearless-cors/#disallow-dangerous-origin-patterns

[aleksasiriski]

I just hit this bug, why isn't it merged yet?

[maxshine]

It is interesting my five years aged PR #57 with the exactly same fix was closed without any endorsements.. lol

[Hvitgar]

I am sorry @maxshine, my PR was not meant to take your contribution away. I only realised after I'd written the PR that you wrote one with the exact same fix

[maxshine]

I am sorry @maxshine, my PR was not meant to take your contribution away. I only realised after I'd written the PR that you wrote one with the exact same fix

@Hvitgar Believe me I understand it is not about your contribution coincides with mine which was ready there for long time.

I just hope this repo owner / maintainers would be able to do their job in a smart way. 😂

[appleboy]

@maxshine, I apologize for not noticing that a similar PR had already been submitted when I was reviewing this PR. This was an oversight on my part, and I will be more careful in the future.

[appleboy]

@maxshine I updated the release note v1.6.0 release note that put the both of @Hvitgar and @maxshine in the list.