From 1a8e32227fb08d428ca5d3a9c362911e0fefe608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Mon, 29 Mar 2021 23:22:58 +0100 Subject: [PATCH] improve "reverse" even further (#289) Fix up a few TODOs, and simplify the way we handle comments. We now add whitespace around inline /*line*/ directives, to ensure we don't break programs. A test case is added too. We now add line directives to call sites, not function declarations, since those are what actually shows up in stack traces. It's unclear if we care about any other lines inside functions at all. This also fixes reversing with -literals, since that feature adds a significant amount of code which shuffles line numbers around. Finally, we extend the tests with types, methods, and anonymous functions, and we make all of them work well. Updates #5. --- main.go | 5 +-- position.go | 57 ++++++++++++++++++++++++----------- reverse.go | 44 +++++++++++++++------------ testdata/scripts/literals.txt | 5 +-- testdata/scripts/position.txt | 10 +++++- testdata/scripts/reverse.txt | 44 +++++++++++++++++++++------ 6 files changed, 113 insertions(+), 52 deletions(-) diff --git a/main.go b/main.go index d87f82d..9cb3e22 100644 --- a/main.go +++ b/main.go @@ -286,7 +286,8 @@ func mainErr(args []string) error { // possibly differentiate between the two versions of a package. // The same happens with "foo/bar_test", whose ImportPath is actually // "foo/bar_test [foo/bar.test]". - // We'll likely file this as an upstream bug to fix in Go 1.17. + // + // TODO(mvdan): remove once https://github.com/golang/go/issues/44963 is fixed // // Until then, here's our workaround: since this edge case only happens // for the compiler, check if any "_test.go" files are being compiled. @@ -582,7 +583,7 @@ func transformCompile(args []string) ([]string, error) { // extra calls to 'go list'. // Since this is a rare edge case and only occurs for a few std // packages, do the extra 'go list' calls for now. - // TODO(mvdan): report this upstream and investigate further. + // TODO(mvdan): remove once https://github.com/golang/go/issues/44630 is fixed if curPkg.Standard && len(curPkg.ImportMap) > 0 { origImporter = importer.Default() } diff --git a/position.go b/position.go index 05f14b9..e255236 100644 --- a/position.go +++ b/position.go @@ -63,21 +63,20 @@ func printFile(file1 *ast.File) ([]byte, error) { addComment := func(offset int, text string) { toAdd = append(toAdd, commentToAdd{offset, text}) } + + // Make sure the entire file gets a zero filename by default, + // in case we miss any positions below. addComment(0, "/*line :1*/") - for _, group := range file2.Comments { - for _, comment := range group.List { - if isDirective(comment.Text) { - // TODO(mvdan): merge with the zeroing below - pos := fset.Position(comment.Pos()) - addComment(pos.Offset, comment.Text) - } - } - } - // Remove all existing comments by making them whitespace. + + // Remove any comments by making them whitespace. + // Keep directives, as they affect the build. // This is superior to removing the comments before printing, // because then the final source would have different line numbers. for _, group := range file2.Comments { for _, comment := range group.List { + if isDirective(comment.Text) { + continue + } start := fset.Position(comment.Pos()).Offset end := fset.Position(comment.End()).Offset for i := start; i < end; i++ { @@ -86,21 +85,36 @@ func printFile(file1 *ast.File) ([]byte, error) { } } - for i, decl := range file2.Decls { + var origCallExprs []*ast.CallExpr + ast.Inspect(file1, func(node ast.Node) bool { + if node, ok := node.(*ast.CallExpr); ok { + origCallExprs = append(origCallExprs, node) + } + return true + }) + + i := 0 + ast.Inspect(file2, func(node ast.Node) bool { + node, ok := node.(*ast.CallExpr) + if !ok { + return true + } + origNode := origCallExprs[i] + i++ newName := "" if !opts.Tiny { - decl := file1.Decls[i] - origPos := fmt.Sprintf("%s:%d", filename, fset.Position(decl.Pos()).Offset) + origPos := fmt.Sprintf("%s:%d", filename, fset.Position(origNode.Pos()).Offset) newName = hashWith(curPkg.GarbleActionID, origPos) + ".go" // log.Printf("%q hashed with %x to %q", origPos, curPkg.GarbleActionID, newName) } newPos := fmt.Sprintf("%s:1", newName) - pos := fset.Position(decl.Pos()) + pos := fset.Position(node.Pos()) - // We use the /*text*/ form, since we can use multiple of them + // We use the "/*text*/" form, since we can use multiple of them // on a single line, and they don't require extra newlines. addComment(pos.Offset, "/*line "+newPos+"*/") - } + return true + }) // We add comments in order. sort.Slice(toAdd, func(i, j int) bool { @@ -111,12 +125,19 @@ func printFile(file1 *ast.File) ([]byte, error) { var buf2 bytes.Buffer for _, comment := range toAdd { buf2.Write(src[copied:comment.offset]) + copied = comment.offset + + // Make sure there is whitespace at either side of a comment. + // Otherwise, we could change the syntax of the program. + // Inserting "/*text*/" in "a/b" // must be "a/ /*text*/ b", + // as "a//*text*/b" is tokenized as a "//" comment. + buf2.WriteByte(' ') buf2.WriteString(comment.text) if strings.HasPrefix(comment.text, "//") { buf2.WriteByte('\n') + } else { + buf2.WriteByte(' ') } - - copied = comment.offset } buf2.Write(src[copied:]) return buf2.Bytes(), nil diff --git a/reverse.go b/reverse.go index e3498bc..03b0ce3 100644 --- a/reverse.go +++ b/reverse.go @@ -62,30 +62,34 @@ func commandReverse(args []string) error { if err != nil { return err } - for _, decl := range file.Decls { + ast.Inspect(file, func(node ast.Node) bool { + switch node := node.(type) { + + // Replace names. + // TODO: do var names ever show up in output? + case *ast.FuncDecl: + addReplace(node.Name.Name) + case *ast.TypeSpec: + addReplace(node.Name.Name) + + case *ast.CallExpr: + // continues below + default: + return true + } + // Reverse position information. - pos := fset.Position(decl.Pos()) + pos := fset.Position(node.Pos()) origPos := fmt.Sprintf("%s:%d", goFile, pos.Offset) - newFilename := hashWith(lpkg.GarbleActionID, origPos) + ".go" + newPos := hashWith(lpkg.GarbleActionID, origPos) + ".go:1" - // For each line number within this function, - // replace the obfuscated position with its original form. - // Remember that the "/*line" directive starts at 1. - numLines := fset.Position(decl.End()).Line - pos.Line - for i := 0; i < numLines; i++ { - replaces = append(replaces, - fmt.Sprintf("%s:%d", newFilename, 1+i), - fmt.Sprintf("%s/%s:%d", lpkg.ImportPath, goFile, pos.Line+i), - ) - } + replaces = append(replaces, + newPos, + fmt.Sprintf("%s/%s:%d", lpkg.ImportPath, goFile, pos.Line), + ) - // Reverse the name itself. - // TODO: Probably do type names too. What else? - switch decl := decl.(type) { - case *ast.FuncDecl: - addReplace(decl.Name.Name) - } - } + return true + }) } } repl := strings.NewReplacer(replaces...) diff --git a/testdata/scripts/literals.txt b/testdata/scripts/literals.txt index f486cd8..25d8b1a 100644 --- a/testdata/scripts/literals.txt +++ b/testdata/scripts/literals.txt @@ -46,8 +46,9 @@ grep '^\s+\w+ := \[\.{3}\](byte|uint16|uint32|uint64)\{[0-9\s,]+\}$' debug1/test # Split obfuscator. Detect decryptKey ^= i * counter grep '^\s+\w+ \^= \w+ \* \w+$' debug1/test/main/extra_literals.go -# XorShuffle obfuscator. Detect data = append(data, x (^|-|+) y...) -grep '^\s+\w+ = append\(\w+,(\s+\w+\[\d+\][\^\-+]\w+\[\d+\],?)+\)$' debug1/test/main/extra_literals.go +# XorShuffle obfuscator. Detect data = append(data, x (^|-|+) y...). +# Note that the line obfuscator adds an inline comment before the call. +grep '^\s+\w+ = .*\bappend\(\w+,(\s+\w+\[\d+\][\^\-+]\w+\[\d+\],?)+\)$' debug1/test/main/extra_literals.go # XorSeed obfuscator. Detect type decFunc func(byte) decFunc grep '^\s+type \w+ func\(byte\) \w+$' debug1/test/main/extra_literals.go diff --git a/testdata/scripts/position.txt b/testdata/scripts/position.txt index 38d68f9..7e2a5a3 100644 --- a/testdata/scripts/position.txt +++ b/testdata/scripts/position.txt @@ -2,7 +2,9 @@ env GOPRIVATE=test/main garble build exec ./main -! stdout 'main.go|other_file_name|is sorted' +! stdout 'main\.go|other_file_name|is sorted' + +! binsubstr main$exe 'main.go' 'other_file_name' [short] stop # no need to verify this with -short @@ -55,6 +57,12 @@ func main() { if sort.IsSorted(sort.StringSlice(varPositions)) { fmt.Println("varPositions is sorted") } + + // Adding "/*text*/" comments here is tricky, + // as we don't want to turn "a/b" into "a//*text*/b". + // We need "a/ /*text*/b" to preserve the syntax. + // The nested expression is needed to prevent spaces. + fmt.Printf("%v\n", 10*float64(3.0)/float64(4.0)) } -- other_file_name.go -- package main diff --git a/testdata/scripts/reverse.txt b/testdata/scripts/reverse.txt index b85e758..61d277c 100644 --- a/testdata/scripts/reverse.txt +++ b/testdata/scripts/reverse.txt @@ -19,13 +19,25 @@ stdin main.stderr garble reverse cmp stdout reverse.stdout -# TODO: test with -literals too, as it modifies the source. - # Ensure that the reversed output matches the non-garbled output. go build -trimpath exec ./main cmp stderr reverse.stdout +# Ensure that we can still reverse with -literals. +garble -literals build +exec ./main +cp stderr main-literals.stderr + +stdin main-literals.stderr +garble -literals reverse +cmp stdout reverse.stdout + +# Reversing a -literals output without the flag should fail. +stdin main-literals.stderr +garble reverse +cmp stdout main-literals.stderr + -- go.mod -- module test/main @@ -44,9 +56,13 @@ func main() { } func unexportedMainFunc() { - if err := lib.ExportedLibFunc(os.Stderr); err != nil { - panic(err) + anonFunc := func() { + lt := lib.ExportedLibType{} + if err := lt.ExportedLibMethod(os.Stderr); err != nil { + panic(err) + } } + anonFunc() } -- lib/lib.go -- package lib @@ -57,7 +73,13 @@ import ( "runtime/debug" ) -func ExportedLibFunc(w io.Writer) error { +type ExportedLibType struct{} + +func (*ExportedLibType) ExportedLibMethod(w io.Writer) error { + return printStackTrace(w) +} + +func printStackTrace(w io.Writer) error { // Panic outputs include "0xNN" pointers and offsets which change // between platforms. // Strip them out here, to have portable static stdout files. @@ -74,9 +96,13 @@ func ExportedLibFunc(w io.Writer) error { goroutine 1 [running]: runtime/debug.Stack(0x??, 0x??, 0x??) runtime/debug/stack.go:24 +0x?? -test/main/lib.ExportedLibFunc(0x??, 0x??, 0x??, 0x??) - test/main/lib/lib.go:17 +0x?? -main.unexportedMainFunc(...) - test/main/main.go:14 +test/main/lib.printStackTrace(0x??, 0x??, 0x??, 0x??) + test/main/lib/lib.go:23 +0x?? +test/main/lib.(*ExportedLibType).ExportedLibMethod(...) + test/main/lib/lib.go:12 +main.unexportedMainFunc.func1() + test/main/main.go:16 +0x?? +main.unexportedMainFunc() + test/main/main.go:20 +0x?? main.main() test/main/main.go:10 +0x??