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

cmd/internal/objfile: Panic with legacy go 1.2 binary #47981

Open
stevemk14ebr opened this issue Aug 26, 2021 · 1 comment
Open

cmd/internal/objfile: Panic with legacy go 1.2 binary #47981

stevemk14ebr opened this issue Aug 26, 2021 · 1 comment
Labels
NeedsInvestigation
Milestone

Comments

@stevemk14ebr
Copy link
Contributor

@stevemk14ebr stevemk14ebr commented Aug 26, 2021

The objfile command internally contains a branch for legacy go binaries. See:

if pclntab, err2 = loadPETable(f.pe, "pclntab", "epclntab"); err2 != nil {
and
data, err := sect.Data()
if err != nil {
return nil, err
}
return data[ssym.Value:esym.Value], nil
. For this path it resolves the go symbols within the '.symtab' named section (for PE files at least). These symbols contain RVAs to index into the '.data' named section (for PE files). In the world there exists binaries where these symbols are malformed, I do not know why, potentially these were edited by hand on purpose, or potentially at some point in time a go compiler bug existed.

The epclntab symbol can in some instances point just beyond the .data section. I'm inclined to think this may be an old bug because subtracting the start address of the .data starting virtual address (4096 in this case) brings the value in range of the end of the data section. The binary I am dealing with is go 1.2

Either way, there is zero verification of the symbol values before indexing into the data array, this should be fixed so that a nice error can be ommited rather than panicking unexpectedly. This also involves my old ticket as well: #42954. For binaries such as this one with invalid symbols the tab can be trivially found with a signature scan. This would be an even better fallback behavior than printing an 'error malformed pclntab symbols'.

@stevemk14ebr
Copy link
Contributor Author

@stevemk14ebr stevemk14ebr commented Aug 26, 2021

Suggested guards (for PE):

func loadPETable(f *pe.File, sname, ename string) ([]byte, error) {
	ssym, err := findPESymbol(f, sname)
	if err != nil {
		return nil, err
	}
	esym, err := findPESymbol(f, ename)
	if err != nil {
		return nil, err
	}
	if ssym.SectionNumber != esym.SectionNumber {
		return nil, fmt.Errorf("%s and %s symbols must be in the same section", sname, ename)
	}

	if uint32(ssym.SectionNumber) > uint32(len(f.Sections)) {
		return nil, fmt.Errorf("pclntab symbol section index out of range")
	}

	sect := f.Sections[ssym.SectionNumber-1]
	data, err := sect.Data()
	if err != nil {
		return nil, err
	}

	if ssym.Value > esym.Value || esym.Value > uint32(len(data)) {
		return nil, fmt.Errorf("pclntab symbols are malformed")
	}

	return data[ssym.Value:esym.Value], nil
}

Similar guards are required for plan9obj.go and xcoff.go

func loadPlan9Table(f *plan9obj.File, sname, ename string) ([]byte, error) {
	ssym, err := findPlan9Symbol(f, sname)
	if err != nil {
		return nil, err
	}
	esym, err := findPlan9Symbol(f, ename)
	if err != nil {
		return nil, err
	}
	sect := f.Section("text")
	if sect == nil {
		return nil, err
	}
	data, err := sect.Data()
	if err != nil {
		return nil, err
	}
	textStart := f.LoadAddress + f.HdrSize

	if textStart > ssym.Value || textStart > esym.Value || esym.Value-textStart > uint64(len(data)) {
		return nil, fmt.Errorf("pclntab symbols are malformed")
	}

	return data[ssym.Value-textStart : esym.Value-textStart], nil
}

@toothrot toothrot changed the title Panic in objfile with legacy go 1.2 binary cmd/internal/objfile: Panic with legacy go 1.2 binary Aug 26, 2021
@toothrot toothrot added this to the Backlog milestone Aug 26, 2021
@toothrot toothrot added the NeedsInvestigation label Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

2 participants