garble icon indicating copy to clipboard operation
garble copied to clipboard

using GORM results in obfuscated table names due to its indirect use of reflection

Open as1543100166 opened this issue 3 years ago • 9 comments

What version of Garble and Go are you using?

$ garble version v0.7.0
$ go version v1.18.3

What did you do?

garble build -v -a -ldflags "-s -w" -trimpath -buildmode=pie

What did you see instead?

build Correct but gorm err

2022/06/17 12:00:00 sqggj5IQ.go:1 Error 1146: Table 'v2_v2.v2_co3i34_y3' doesn't exist
SELECT sum(u+d) as n,u,d,user,record,updated,server FROM `v2_co3i34_y3`.....

as1543100166 avatar Jun 17 '22 07:06 as1543100166

Can you provide some source code to reproduce this problem in isolation? I'm not familiar with gorm to know how to reproduce this straight away.

mvdan avatar Jun 17 '22 10:06 mvdan

Can you provide some source code to reproduce this problem in isolation? I'm not familiar with gorm to know how to reproduce this straight away.

After trying, this problem was still found

garble build -v -a -ldflags "-s -w" -trimpath -buildmode=pie
type StatUser struct {
	Id      int64 `gorm:"primaryKey"`
	User_Id int64
}
func main() {

	dsn := fmt.Sprintf("%s:%s@tcp(%s:%v)/%s?charset=utf8mb4&parseTime=True&loc=Local", "test", "passwd", "localhost", 3306, "mydb")
	gormConfig := &gorm.Config{
		//Logger: logger.Default.LogMode(logger.Silent),
		NamingStrategy: schema.NamingStrategy{
			TablePrefix:   "v1_",
			SingularTable: true,
		},
	}
	db, err := gorm.Open(mysql.Open(dsn), gormConfig)
	if err != nil {
		fmt.Printf("err: %v", err)
	}
	sqlDB, _ := db.DB()
	sqlDB.SetMaxIdleConns(10)
	sqlDB.SetMaxOpenConns(100)
	sqlDB.SetConnMaxLifetime(10 * time.Second)

	var suser []StatUser
	db.Where("record_at >= ? and updated_at >= ?", "123456", "123456").Select("sum(u+d) as n", "u", "d", "user_id", "record_at", "updated_at", "server_rate").Group("user_id").Order("n DESC").Limit(8).Find(&suser)
}

as1543100166 avatar Jun 17 '22 12:06 as1543100166

Thanks. It took some effort to actually run that, because it's not self-contained - it lacked import paths, and I don't have a mysql database, so I swapped that for sqlite.

$ cat main.go
package main

import (
	"gorm.io/driver/sqlite"
	"gorm.io/gorm"
)

type StatUser struct {
	Id      int64 `gorm:"primaryKey"`
	User_Id int64
}

func main() {
	db, err := gorm.Open(sqlite.Open("foo.sqlite"), nil)
	if err != nil {
		panic(err)
	}
	var suser StatUser
	db.Where("record_at >= 123").Select("user_id").Find(&suser)
}
$ go build && ./test

2022/06/17 15:38:07 /tmp/tmp.pEilU71vL8/main.go:19 no such table: stat_users
[0.069ms] [rows:0] SELECT `user_id` FROM `stat_users` WHERE record_at >= 123
$ garble build && ./test

2022/06/17 15:38:11 VVlHTDz0.go:1 no such table: odo_dwp_vbs
[0.068ms] [rows:0] SELECT user_id FROM `odo_dwp_vbs` WHERE record_at >= 123

The above shows the problem; without obfuscation, the table name is stat_users, but with obfuscation it's something else, derived from the obfuscated type name.

garble tries to detect direct and indirect uses of reflection, but in this particular case, GORM isn't obvious about the fact that it uses reflection:

https://github.com/go-gorm/gorm/blob/1305f637f834baa13c514df915157a51d86b4f28/finisher_api.go#L169

Note how the call only stores the interface value. It's only used with reflection at a later point, which we can't track given the field.

Your immediate fix would be to add var _ = reflect.TypeOf(StatUser{}), as described in the README. The only alternative I can think of right now would be to hard-code the knowledge in garble that these GORM APIs use reflection; but I would prefer not to do that, because such a hard-coded list will always be incomplete.

mvdan avatar Jun 17 '22 14:06 mvdan

I think its still detectable with some effort, I will try in the next few days.

lu4p avatar Jun 17 '22 15:06 lu4p

As an incremental step, it might be enough to record that the field Statement.Dest is used for reflection later on. Beware that this will be a slippery slope - to properly cover all edge cases, you would have to add an insane amount of Go code to track assignments, variables, function calls, addresses and pointers, and so on.

The problem we're facing with detecting reflection is essentially https://en.wikipedia.org/wiki/Symbolic_execution. We want to understand how some Go code would behave without actually running it, just via static analysis.

If we truly want to go down that path, then we should really be using SSA, not the AST. I'm not saying we need to do that now - if anything, it seems like we should just tell people to use the workaround for now, because we have other areas of work that don't have workarounds like these, like being able to obfuscate the runtime. This is just a warning that properly tracking what types/values get used for reflection is a rabbit hole of research and expensive computation.

mvdan avatar Jun 17 '22 15:06 mvdan

Thanks for the warning, however I think its not that complicated when just blacklisting parameters on a function level.

lu4p avatar Jun 17 '22 16:06 lu4p

this should work :

var _ = reflect.TypeOf(StatUser{}) type StatUser struct { Id int64 gorm:"primaryKey" User_Id int64 } func (StatUser) TableName() string { return "stat_users" }

alexmaloteaux avatar Jun 20 '22 20:06 alexmaloteaux

I imagine you only need to either add the TypeOf hint for garble, so that the struct type isn't obfuscated, or add the TableName method so that the table name doesn't come from the type name. I imagine you don't need both workarounds at the same time.

mvdan avatar Jun 20 '22 20:06 mvdan

I imagine you only need to either add the TypeOf hint for garble, so that the struct type isn't obfuscated, or add the TableName method so that the table name doesn't come from the type name. I imagine you don't need both workarounds at the same time.

up to a certain version of garble TableName was enough, then Typeof was mandatory, so for historical reason i use both :) but TableName only would fail for sure

alexmaloteaux avatar Jun 20 '22 21:06 alexmaloteaux

Any updates on this?

We are getting obfuscated table names which will work just fine but after a new release the database no longer finds that tables because the obfuscated table names and columns change.

[1.576ms] [rows:0] SELECT * FROM ub_v_sz_bnsWHEREub_v_sz_bns.ha_qbb_ei_= "server.servers.com" ORDER BYub_v_sz_bns.zux_yzz_af LIMIT 1

As a workfound I added table names to structs and tags for hinting column names.

matiniamirhossein avatar Sep 22 '22 12:09 matiniamirhossein

@matiniamirhossein does your case look like my reproducer example in https://github.com/burrowers/garble/issues/554#issuecomment-1158942332? If not, how is it different? I show a workaround in that comment via reflect.TypeOf.

mvdan avatar Sep 22 '22 14:09 mvdan

Below is a working example to show that reflect.TypeOf and TableName are both valid workarounds for this problem:

$ go version
go version go1.19.1 linux/amd64
$ garble version
mvdan.cc/garble v0.7.2-0.20220906164632-f9d99190d29e

Build settings:
      -buildmode exe
       -compiler gc
        -ldflags -w -s
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v3
$ testscript -e GOMODCACHE=$(go env GOMODCACHE) -v repro.txtar
[...]
> exec go build -trimpath main_normal.go
> exec ./main_normal
[stdout]

2022/09/22 15:26:59 ./main_normal.go:19 no such table: stat_users
[0.138ms] [rows:0] SELECT `user_id` FROM `stat_users` WHERE record_at >= 123
> exec garble build main_normal.go
> exec ./main_normal
[stdout]

2022/09/22 15:27:31 aSJPP4LZ.go:1 no such table: b_rb8tns_us
[0.102ms] [rows:0] SELECT user_id FROM `b_rb8tns_us` WHERE record_at >= 123
> exec go build -trimpath main_reflect.go
> exec ./main_reflect
[stdout]

2022/09/22 15:27:32 ./main_reflect.go:23 no such table: stat_users
[0.085ms] [rows:0] SELECT `user_id` FROM `stat_users` WHERE record_at >= 123
> exec garble build main_reflect.go
> exec ./main_reflect
[stdout]

2022/09/22 15:27:33 G8pCurLs.go:1 no such table: stat_users
[0.083ms] [rows:0] SELECT `user_id` FROM `stat_users` WHERE record_at >= 123
> exec go build -trimpath main_tablename.go
> exec ./main_tablename
[stdout]

2022/09/22 15:27:33 ./main_tablename.go:23 no such table: my_table_name
[0.082ms] [rows:0] SELECT `user_id` FROM `my_table_name` WHERE record_at >= 123
> exec garble build main_tablename.go
> exec ./main_tablename
[stdout]

2022/09/22 15:27:34 JYlIfkQI.go:1 no such table: my_table_name
[0.097ms] [rows:0] SELECT user_id FROM `my_table_name` WHERE record_at >= 123
PASS

Note how the row names are different in my normal example, but when using TypeOf or TableName, they remain the same between the regular and obfuscated build. I used https://pkg.go.dev/github.com/rogpeppe/go-internal/cmd/testscript with the file below if anyone wants to give it a go. The three main files are exactly the same, except that the second and third files add the following lines respectively:

  • var _ = reflect.TypeOf(StatUser{})
  • func (StatUser) TableName() string { return "my_table_name" }
exec go mod tidy

exec go build -trimpath main_normal.go
exec ./main_normal

exec garble build main_normal.go
exec ./main_normal

exec go build -trimpath main_reflect.go
exec ./main_reflect

exec garble build main_reflect.go
exec ./main_reflect

exec go build -trimpath main_tablename.go
exec ./main_tablename

exec garble build main_tablename.go
exec ./main_tablename

-- go.mod --
module test

go 1.18

require (
	gorm.io/driver/sqlite v1.3.6
	gorm.io/gorm v1.23.10
)

require (
	github.com/jinzhu/inflection v1.0.0 // indirect
	github.com/jinzhu/now v1.1.5 // indirect
	github.com/mattn/go-sqlite3 v1.14.12 // indirect
)
-- main_normal.go --
package main

import (
	"gorm.io/driver/sqlite"
	"gorm.io/gorm"
)

type StatUser struct {
	Id      int64 `gorm:"primaryKey"`
	User_Id int64
}

func main() {
	db, err := gorm.Open(sqlite.Open("foo.sqlite"), nil)
	if err != nil {
		panic(err)
	}
	var suser StatUser
	db.Where("record_at >= 123").Select("user_id").Find(&suser)
}
-- main_reflect.go --
package main

import (
	"reflect"

	"gorm.io/driver/sqlite"
	"gorm.io/gorm"
)

type StatUser struct {
	Id      int64 `gorm:"primaryKey"`
	User_Id int64
}

var _ = reflect.TypeOf(StatUser{})

func main() {
	db, err := gorm.Open(sqlite.Open("foo.sqlite"), nil)
	if err != nil {
		panic(err)
	}
	var suser StatUser
	db.Where("record_at >= 123").Select("user_id").Find(&suser)
}
-- main_tablename.go --
package main

import (
	"gorm.io/driver/sqlite"
	"gorm.io/gorm"
)

type StatUser struct {
	Id      int64 `gorm:"primaryKey"`
	User_Id int64
}

func (StatUser) TableName() string {
	return "my_table_name"
}

func main() {
	db, err := gorm.Open(sqlite.Open("foo.sqlite"), nil)
	if err != nil {
		panic(err)
	}
	var suser StatUser
	db.Where("record_at >= 123").Select("user_id").Find(&suser)
}

So, yes, there is a bug in garble - but there are two rather easy workarounds as far as I am aware.

mvdan avatar Sep 22 '22 14:09 mvdan