Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse issue on go-ethereum: undeclared name: max_path_size #555

Closed
zcrypt0 opened this issue Jun 17, 2022 · 2 comments · Fixed by #584
Closed

Reverse issue on go-ethereum: undeclared name: max_path_size #555

zcrypt0 opened this issue Jun 17, 2022 · 2 comments · Fixed by #584

Comments

@zcrypt0
Copy link

zcrypt0 commented Jun 17, 2022

What version of Garble and Go are you using?

Custom, forked from master, to add a blacklist argument.

$ garble version
https://github.com/zcrypt0/garble/tree/72792ae964f200c6741036ef38b08f13c23ec16a

$ go version v1.18.3

What environment are you running Garble on?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/go-ethereum/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3469986570=/tmp/go-build -gno-record-gcc-switches"

What did you do?

See below for comments on the custom -blacklist argument.

$ go install github.com/zcrypt0/garble@72792ae964f200c6741036ef38b08f13c23ec16a
$ git clone https://github.com/ethereum/go-ethereum
$ cd go-ethereum
$ touch ./panic-output.txt
$ garble -blacklist github.com/ethereum/go-ethereum/crypto/bn256/cloudflare build -trimpath -v -o ./build/bin/geth ./cmd/geth
$ garble -blacklist github.com/ethereum/go-ethereum/crypto/bn256/cloudflare reverse ./cmd/geth ./panic-output.txt

What did you expect to see?

Successfully reverse an obfuscated panic trace.

What did you see instead?

Reversing fails on the rpc subpackage (github.com/ethereum/go-ethereum/rpc), no actual panic output is required.

typecheck error: /go-ethereum/rpc/ipc_unix.go:34:25: undeclared name: max_path_size

Comments

The blacklist on the custom version simply ensures that pkg.ToObfuscate is false for an obfuscated package and can be replicated without using my fork of garble by simply making a build that adds the blacklisted package to the cannotObfuscate map.

The blacklisted package is necessary to avoid #553

The var max_path_size is a package level var declared in another file and reverse (not build) seems to have some issue with it. Looks like it makes a call out to C for its value.

https://github.com/ethereum/go-ethereum/blob/master/rpc/ipc_unix.go#L34
https://github.com/ethereum/go-ethereum/blob/master/rpc/constants_unix.go#L33

@mvdan
Copy link
Member

mvdan commented Jun 17, 2022

I'll focus on fixing #553 first, for the sake of being able to reproduce this bug on master and not with a fork.

mvdan added a commit to mvdan/garble-fork that referenced this issue Sep 23, 2022
The reverse feature relied on `GoFiles` from `go list`,
but that list may not be enough to typecheck a package:

	typecheck error: $WORK/main.go:3:15: undeclared name: longMain

`go help list` shows:

	GoFiles         []string   // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
	CgoFiles        []string   // .go source files that import "C"
	CompiledGoFiles []string   // .go files presented to compiler (when using -compiled)

In other words, to mimic the same list of Go files fed to the compiler,
we want CompiledGoFiles.

Note that, since the cgo files show up as generated files,
we currently do not support reversing their filenames.
That is left as a TODO for now.

Updates burrowers#555.
mvdan added a commit to mvdan/garble-fork that referenced this issue Sep 23, 2022
See https://golang.org/issue/28749. The improved asm test would fail:

	go parse: $WORK/imported/imported_amd64.s:1:1: expected 'package', found TEXT (and 2 more errors)

because we would incorrectly parse a non-Go file as a Go file.

Add a workaround. The original reporter's reproducer with go-ethereum
works now, as this was the last hiccup.

Fixes burrowers#555.
@mvdan
Copy link
Member

mvdan commented Sep 23, 2022

#553 should have been fixed in master, and this second bug should be fixed by #584. Feel free to give that a go :)

mvdan added a commit that referenced this issue Sep 25, 2022
The reverse feature relied on `GoFiles` from `go list`,
but that list may not be enough to typecheck a package:

	typecheck error: $WORK/main.go:3:15: undeclared name: longMain

`go help list` shows:

	GoFiles         []string   // .go source files (excluding CgoFiles, TestGoFiles, XTestGoFiles)
	CgoFiles        []string   // .go source files that import "C"
	CompiledGoFiles []string   // .go files presented to compiler (when using -compiled)

In other words, to mimic the same list of Go files fed to the compiler,
we want CompiledGoFiles.

Note that, since the cgo files show up as generated files,
we currently do not support reversing their filenames.
That is left as a TODO for now.

Updates #555.
mvdan added a commit that referenced this issue Sep 25, 2022
See https://golang.org/issue/28749. The improved asm test would fail:

	go parse: $WORK/imported/imported_amd64.s:1:1: expected 'package', found TEXT (and 2 more errors)

because we would incorrectly parse a non-Go file as a Go file.

Add a workaround. The original reporter's reproducer with go-ethereum
works now, as this was the last hiccup.

Fixes #555.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants