From 7c509798997e48adc5e3ecc2bffcb1fc1846251c Mon Sep 17 00:00:00 2001 From: pagran Date: Tue, 31 Jan 2023 20:11:59 +0100 Subject: [PATCH] ensure that all imports are used after obfuscation garble's -literals flag and its patching of the runtime may leave unused imports. We used to try to detect those and remove the imports, but that was still buggy with edge cases like dot imports or renamed imports. Moreover, it was potentially incorrect. Completely removing an import from a package means we don't run its init funcs, which could have side effects changing the behavior of a program. As an example, database/sql drivers are registered at init time. Instead, for each import in an obfuscated Go file, add an unnamed declaration which references the imported package. This may not be necessary for all imported packages, as only a minority become unused due to garble, but it's also relatively harmless to do so. Fixes #658. --- main.go | 147 +++++++++++++++++++++++++-------- testdata/script/literals.txtar | 64 ++++++++++++++ 2 files changed, 177 insertions(+), 34 deletions(-) diff --git a/main.go b/main.go index cdc8f2f..d3650e0 100644 --- a/main.go +++ b/main.go @@ -40,7 +40,6 @@ import ( "golang.org/x/mod/module" "golang.org/x/mod/semver" "golang.org/x/tools/go/ast/astutil" - "mvdan.cc/garble/internal/linker" "mvdan.cc/garble/internal/literals" ) @@ -951,7 +950,7 @@ func transformCompile(args []string) ([]string, error) { if flagTiny { // strip unneeded runtime code stripRuntime(basename, file) - tf.removeUnnecessaryImports(file) + tf.makeImportsUsed(file) } if basename == "symtab.go" { updateMagicValue(file, magicValue()) @@ -1514,9 +1513,10 @@ type transformer struct { func newTransformer() *transformer { return &transformer{ info: &types.Info{ - Types: make(map[ast.Expr]types.TypeAndValue), - Defs: make(map[*ast.Ident]types.Object), - Uses: make(map[*ast.Ident]types.Object), + Types: make(map[ast.Expr]types.TypeAndValue), + Defs: make(map[*ast.Ident]types.Object), + Uses: make(map[*ast.Ident]types.Object), + Implicits: make(map[ast.Node]types.Object), }, recordTypeDone: make(map[*types.Named]bool), fieldToStruct: make(map[*types.Var]*types.Struct), @@ -1687,48 +1687,127 @@ func recordedAsNotObfuscated(obj types.Object) bool { return ok } -func (tf *transformer) removeUnnecessaryImports(file *ast.File) { - usedImports := make(map[string]bool) - ast.Inspect(file, func(n ast.Node) bool { - node, ok := n.(*ast.Ident) - if !ok { - return true - } +func (tf *transformer) resolveImportSpec(imp *ast.ImportSpec) *types.Package { + // Simple import has no ast.Ident and is stored in Implicits separately. + obj := tf.info.Implicits[imp] + if obj == nil { + obj = tf.info.Defs[imp.Name] // renamed or dot import + } + if obj == nil { + return nil + } - uses, ok := tf.info.Uses[node] - if !ok { - return true - } + pkgObj, ok := obj.(*types.PkgName) + if !ok { + panic(fmt.Sprintf("unexpected object type for %s: %v", imp.Path.Value, obj)) + } + return pkgObj.Imported() +} - if pkg := uses.Pkg(); pkg != nil { - usedImports[pkg.Path()] = true +// isSafeForInstanceType returns true if the passed type is safe for var declaration. +// Unsafe types: generic types and non-method interfaces. +func isSafeForInstanceType(typ types.Type) bool { + switch t := typ.(type) { + case *types.Named: + if t.TypeParams().Len() > 0 { + return false } + return isSafeForInstanceType(t.Underlying()) + case *types.Signature: + return t.TypeParams().Len() == 0 + case *types.Interface: + return t.IsMethodSet() + } + return true +} - return true - }) - +func (tf *transformer) makeImportsUsed(file *ast.File) { for _, imp := range file.Imports { if imp.Name != nil && imp.Name.Name == "_" { continue } - path, err := strconv.Unquote(imp.Path.Value) - if err != nil { - panic(err) + pkg := tf.resolveImportSpec(imp) + if pkg == nil { + panic(fmt.Sprintf("import %s not found", imp.Path.Value)) } - // The import path can't be used directly here, because the actual - // path resolved via go/types might be different from the naive path. - lpkg, err := listPackage(path) - if err != nil { - panic(err) - } + generated := false + scope := pkg.Scope() + for _, name := range scope.Names() { + if !token.IsExported(name) { + continue + } + obj := scope.Lookup(name) + if obj == nil { + panic(fmt.Sprintf("%s not found in %s", name, imp.Path.Value)) + } + if !isSafeForInstanceType(obj.Type()) { + continue + } - if usedImports[lpkg.ImportPath] { - continue + nameIdent := ast.NewIdent(name) + getFullName := func() ast.Expr { + if imp.Name == nil { + return &ast.SelectorExpr{ + X: ast.NewIdent(pkg.Name()), + Sel: nameIdent, + } + } + if imp.Name.Name == "." { + return nameIdent + } + return &ast.SelectorExpr{ + X: ast.NewIdent(imp.Name.Name), + Sel: nameIdent, + } + } + + var decl *ast.GenDecl + switch obj.(type) { + case *types.Const: + // const _ = + decl = &ast.GenDecl{ + Tok: token.CONST, + Specs: []ast.Spec{&ast.ValueSpec{ + Names: []*ast.Ident{ast.NewIdent("_")}, + Values: []ast.Expr{getFullName()}, + }}, + } + case *types.Var, *types.Func: + // var _ = + decl = &ast.GenDecl{ + Tok: token.VAR, + Specs: []ast.Spec{&ast.ValueSpec{ + Names: []*ast.Ident{ast.NewIdent("_")}, + Values: []ast.Expr{getFullName()}, + }}, + } + case *types.TypeName: + // var _ + decl = &ast.GenDecl{ + Tok: token.VAR, + Specs: []ast.Spec{&ast.ValueSpec{ + Names: []*ast.Ident{ast.NewIdent("_")}, + Type: getFullName(), + }}, + } + default: + continue // Skip *types.Builtin and other + } + + // Manually add a new variable for correct name obfuscation + tf.info.Uses[nameIdent] = obj + file.Decls = append(file.Decls, decl) + generated = true + break } - imp.Name = ast.NewIdent("_") + if !generated { + // A very unlikely situation where there is no suitable declaration for a reference variable + // and almost certainly means that there is another import reference in code. + log.Printf("generate reference variable for %s failed", imp.Path.Value) + } } } @@ -1744,7 +1823,7 @@ func (tf *transformer) transformGoFile(file *ast.File) *ast.File { file = literals.Obfuscate(obfRand, file, tf.info, tf.linkerVariableStrings) // some imported constants might not be needed anymore, remove unnecessary imports - tf.removeUnnecessaryImports(file) + tf.makeImportsUsed(file) } pre := func(cursor *astutil.Cursor) bool { diff --git a/testdata/script/literals.txtar b/testdata/script/literals.txtar index 07dc7ae..78f8767 100644 --- a/testdata/script/literals.txtar +++ b/testdata/script/literals.txtar @@ -66,6 +66,7 @@ import ( . "test/main/imp_func" . "test/main/imp_var" . "test/main/imp_type" + . "test/main/imp_struct" ) type structTest struct { @@ -145,6 +146,7 @@ func main() { byteTest() shadowTest() dotImportTest() + multipleTimeImportTest() strArray := [2]string{"1: literal in", "an secret array"} println(strArray[0], strArray[1]) @@ -330,7 +332,56 @@ func dotImportTest() { println(DotImportedFunc()) println(DotImportedVar) println(DotImportedType("str as dot imported type")) + println(DotImportedStruct.Str) } + +func multipleTimeImportTest() { + regularAndUnusedRenamed() + regularAndUnusedDotImport() + unusedRegularAndDotImport() +} + +func noop(...any) {} + +-- regular_and_unused_renamed.go -- +package main + +import ( + "test/main/imp_mult" + imp_mult2 "test/main/imp_mult" +) + +func regularAndUnusedRenamed() { + imp_mult.MultDummy() + noop(imp_mult.MultImpStr, imp_mult2.MultImpStr) +} + +-- regular_and_unused_dotimport.go -- +package main + +import ( + "test/main/imp_mult" + . "test/main/imp_mult" +) + +func regularAndUnusedDotImport() { + imp_mult.MultDummy() + noop(imp_mult.MultImpStr, MultImpStr) +} + +-- unused_regular_and_dotimport.go -- +package main + +import ( + "test/main/imp_mult" + . "test/main/imp_mult" +) + +func unusedRegularAndDotImport() { + MultDummy() + noop(imp_mult.MultImpStr, MultImpStr) +} + -- imp/imported.go -- package imported @@ -357,6 +408,18 @@ package imp_type type DotImportedType string +-- imp_struct/imported.go -- +package imp_struct + +var DotImportedStruct = struct{ Str string }{Str: "string in dot imported struct"} + +-- imp_mult/imported.go -- +package imp_mult + +const MultImpStr = "str from package imported multiple time" + +func MultDummy() {} + -- directives.go -- // If we misplace any of the directives below, // cmd/compile will complain with "misplaced compiler directive". @@ -445,5 +508,6 @@ const str from dot imported var str from dot imported func str from dot imported var str as dot imported type +string in dot imported struct 1: literal in an secret array 2: literal in a secret slice