lightwalletd
lightwalletd copied to clipboard
Make it more clear to auditors that security-critical regexp can't match strings with newlines
Although the suggested change is okay, my experiments show this is not a problem. The Golang regexp
documentation is here: https://github.com/google/re2/wiki/Syntax. I wrote a small test program to try to reproduce the problem (you can click on Run to run the program):
https://play.golang.org/p/NrgjszRg-9g
As you can see, the pattern doesn't match (match
is false). On further investigation, I found that the documentation says that ^
and $
match the beginning and end of any line (within a possibly much longer string) only if "multi-line" mode is enabled (search within that page for "multi-line"). If we modify the test program to enable multi-line mode (the pattern string begins with (?m)
, which is how the multi-line mode flag is specified):
https://play.golang.org/p/edqhoeeVqon
As you can see if you run this program, the pattern now matches. Here is that program, in case the above playground link stops working:
package main
import (
"fmt"
"regexp"
)
func main() {
match, err := regexp.Match("(?m)^t[a-zA-Z0-9]{3}$", []byte("taaa\nthis is a separate line"))
fmt.Println(match, err)
}
Because our code doesn't specify the multi-line flag, I think the pattern matching as written (with ^
and $
) works as desired.
Thanks @LarryRuane! I think it's worth using \A
and \z
so it's obvious there's no problem, I reworded the commit message to indicate that.
@defuse, sorry, I forgot about this. Do you still think it's worth doing? If so, you could reopen it and I could quickly merge it.