Skip to content

Commit

Permalink
Added logic to remove existing executable if it can't be truncated
Browse files Browse the repository at this point in the history
  • Loading branch information
oxtoacart committed Feb 19, 2015
1 parent ce61fa2 commit 3f984ae
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion byteexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func New(data []byte, filename string) (*Exec, error) {
}

log.Tracef("Data in %s doesn't match expected, truncating file", filename)
file, err = os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
file, err = openAndTruncate(filename, true)
if err != nil {
return nil, fmt.Errorf("Unable to truncate %s: %s", filename, err)
}
Expand All @@ -110,6 +110,20 @@ func New(data []byte, filename string) (*Exec, error) {
return newExec(filename)
}

func openAndTruncate(filename string, removeIfNecessary bool) (*os.File, error) {
file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fileMode)
if err != nil && os.IsPermission(err) && removeIfNecessary {
log.Tracef("Permission denied truncating file %v, try to remove", filename)
err = os.Remove(filename)
if err != nil {
return nil, fmt.Errorf("Unable to remove file %v: %v", filename, err)
}
return openAndTruncate(filename, false)
}

return file, err
}

// Command creates an exec.Cmd using the supplied args.
func (be *Exec) Command(args ...string) *exec.Cmd {
return exec.Command(be.Filename, args...)
Expand Down

5 comments on commit 3f984ae

@oxtoacart
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fffw I don't know if this is the bug that you were talking about, but I noticed during testing that I couldn't overwrite the pac program once the owner had been changed to root:wheel.

@oxtoacart
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the problem.

@fffw
Copy link

@fffw fffw commented on 3f984ae Feb 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oxtoacart Yes that's what I found. I think we can always remove the file to be written if it already exists.

@oxtoacart
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can always remove the file to be written if it already exists.

If we have write permissions to the file but not the containing directory, we can truncate the file but not remove it.

Does the code as written look OK to you?

@fffw
Copy link

@fffw fffw commented on 3f984ae Feb 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. LGTM.

Please sign in to comment.