mirror of
https://github.com/mandiant/gopacket
synced 2026-06-21 13:57:02 +00:00
exec-tools: fix output polling for long-running commands
Two compounding bugs in wmiexec / atexec / smbexec / dcomexec made the output-retrieval loop return early on any command that didn't finish within the first 100ms poll, leaking the temp file in ADMIN$ / C$. Issue #22 reports the symptom for wmiexec (systeminfo, ping); the same broken pattern existed in all four tools. 1. Wrong call site. The polling loop treated a sharing-violation on smbClient.Cat as the signal "command still running." Cat opens with FILE_SHARE_READ-compatible access — it succeeds against the writer on the first poll and returns the file as it currently exists (typically empty). The conflict actually lives on Rm: deleting the file requires DELETE access, which conflicts with the writer's share mode. Moved the sharing- violation check from Cat to Rm. Matches Impacket's wmiexec.py. 2. Dead string match. The check was strings.Contains(err.Error(), "STATUS_SHARING_VIOLATION"). The underlying smb2 library's NtStatus.Error() returns only the human-readable description ("A file cannot be opened because the share access flags are incompatible.") — the literal constant name never appears in err.Error(), so the check could never fire. Same issue for STATUS_OBJECT_NAME_NOT_FOUND in the not-found branch (further muddled by smb2/conn.go translating that NTSTATUS to os.ErrNotExist before wrapping). Added typed helpers in pkg/smb (IsSharingViolation, IsNotFound) that unwrap through *os.PathError and match on *smb2.ResponseError.Code plus the os.ErrNotExist sentinel. NTSTATUS values are hardcoded since the smb2/internal/erref package can't be imported from outside the smb2 tree. The dcomexec "broken / connection reset / use of closed" branch stays string-matched — those errors come from net, not smb2. Thanks to @aimogging in #22 for the diagnosis and proof-of-concept in #29; this change applies the same Cat-vs-Rm insight across all four exec tools and replaces the err.Error() substring matching with typed-error helpers so future smb2 library changes don't silently break the check again. Verified against GOAD winterfell: wmiexec whoami / systeminfo / ping -n 4 1.1.1.1 all return full output, exit 0, no ADMIN$ residue, both directly from the operator box and over SOCKS5H proxy.
This commit is contained in:
@@ -0,0 +1,65 @@
|
|||||||
|
// Copyright 2026 Google LLC
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// https://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
//
|
||||||
|
// Unless required by applicable law or agreed to in writing, software
|
||||||
|
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
// See the License for the specific language governing permissions and
|
||||||
|
// limitations under the License.
|
||||||
|
|
||||||
|
package smb
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"os"
|
||||||
|
|
||||||
|
"github.com/mandiant/gopacket/pkg/third_party/smb2"
|
||||||
|
)
|
||||||
|
|
||||||
|
// NTSTATUS values from [MS-ERREF]. The smb2 erref package is internal, so we
|
||||||
|
// match the codes on *smb2.ResponseError.Code directly.
|
||||||
|
const (
|
||||||
|
ntStatusObjectNameNotFound uint32 = 0xC0000034
|
||||||
|
ntStatusObjectPathNotFound uint32 = 0xC000003A
|
||||||
|
ntStatusSharingViolation uint32 = 0xC0000043
|
||||||
|
)
|
||||||
|
|
||||||
|
// IsSharingViolation reports whether err wraps STATUS_SHARING_VIOLATION.
|
||||||
|
// The smb2 client wraps server responses in *os.PathError → *smb2.ResponseError,
|
||||||
|
// so substring matching on err.Error() is unreliable.
|
||||||
|
func IsSharingViolation(err error) bool {
|
||||||
|
return ntStatusIs(err, ntStatusSharingViolation)
|
||||||
|
}
|
||||||
|
|
||||||
|
// IsNotFound reports whether err means the SMB target file or path does not
|
||||||
|
// exist. The smb2 layer translates STATUS_OBJECT_NAME_NOT_FOUND and
|
||||||
|
// STATUS_OBJECT_PATH_NOT_FOUND to os.ErrNotExist in conn.go before wrapping,
|
||||||
|
// so most call sites see the sentinel rather than an *smb2.ResponseError.
|
||||||
|
// We accept both forms.
|
||||||
|
func IsNotFound(err error) bool {
|
||||||
|
if errors.Is(err, os.ErrNotExist) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
return ntStatusIs(err, ntStatusObjectNameNotFound) ||
|
||||||
|
ntStatusIs(err, ntStatusObjectPathNotFound)
|
||||||
|
}
|
||||||
|
|
||||||
|
func ntStatusIs(err error, want uint32) bool {
|
||||||
|
if err == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
var pe *os.PathError
|
||||||
|
if errors.As(err, &pe) {
|
||||||
|
err = pe.Err
|
||||||
|
}
|
||||||
|
var re *smb2.ResponseError
|
||||||
|
if errors.As(err, &re) {
|
||||||
|
return re.Code == want
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
@@ -0,0 +1,63 @@
|
|||||||
|
// Copyright 2026 Google LLC
|
||||||
|
//
|
||||||
|
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
// you may not use this file except in compliance with the License.
|
||||||
|
// You may obtain a copy of the License at
|
||||||
|
//
|
||||||
|
// https://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
package smb
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/mandiant/gopacket/pkg/third_party/smb2"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestIsSharingViolation(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
err error
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{"nil", nil, false},
|
||||||
|
{"plain error", errors.New("boom"), false},
|
||||||
|
{"raw response error", &smb2.ResponseError{Code: ntStatusSharingViolation}, true},
|
||||||
|
{"wrapped in PathError", &os.PathError{Op: "remove", Path: "x", Err: &smb2.ResponseError{Code: ntStatusSharingViolation}}, true},
|
||||||
|
{"different NTSTATUS", &os.PathError{Op: "remove", Path: "x", Err: &smb2.ResponseError{Code: ntStatusObjectNameNotFound}}, false},
|
||||||
|
{"fmt-wrapped", fmt.Errorf("retrieval: %w", &smb2.ResponseError{Code: ntStatusSharingViolation}), true},
|
||||||
|
}
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
if got := IsSharingViolation(tc.err); got != tc.want {
|
||||||
|
t.Errorf("IsSharingViolation(%v) = %v, want %v", tc.err, got, tc.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestIsNotFound(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
err error
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
{"nil", nil, false},
|
||||||
|
{"name not found", &os.PathError{Op: "open", Path: "x", Err: &smb2.ResponseError{Code: ntStatusObjectNameNotFound}}, true},
|
||||||
|
{"path not found", &os.PathError{Op: "open", Path: "x", Err: &smb2.ResponseError{Code: ntStatusObjectPathNotFound}}, true},
|
||||||
|
{"os.ErrNotExist sentinel (smb2 translates name_not_found here)", &os.PathError{Op: "open", Path: "__output", Err: os.ErrNotExist}, true},
|
||||||
|
{"bare os.ErrNotExist", os.ErrNotExist, true},
|
||||||
|
{"sharing violation", &os.PathError{Op: "remove", Path: "x", Err: &smb2.ResponseError{Code: ntStatusSharingViolation}}, false},
|
||||||
|
{"unrelated error", errors.New("no such file"), false},
|
||||||
|
}
|
||||||
|
for _, tc := range tests {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
if got := IsNotFound(tc.err); got != tc.want {
|
||||||
|
t.Errorf("IsNotFound(%v) = %v, want %v", tc.err, got, tc.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -322,18 +322,16 @@ func (e *AtExec) getOutput(tmpFileName string) string {
|
|||||||
|
|
||||||
content, err := e.smbClient.Cat(outputPath)
|
content, err := e.smbClient.Cat(outputPath)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
// Delete the output file
|
// Cat succeeds even while the remote process is still writing;
|
||||||
e.smbClient.Rm(outputPath)
|
// Rm is what fails with a sharing violation in that case.
|
||||||
|
if rmErr := e.smbClient.Rm(outputPath); rmErr != nil && smb.IsSharingViolation(rmErr) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
return content
|
return content
|
||||||
}
|
}
|
||||||
|
|
||||||
// If sharing violation, command is still running
|
// File not yet created — keep waiting.
|
||||||
if strings.Contains(err.Error(), "STATUS_SHARING_VIOLATION") {
|
if smb.IsNotFound(err) {
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
// If file not found, keep waiting
|
|
||||||
if strings.Contains(err.Error(), "STATUS_OBJECT_NAME_NOT_FOUND") {
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+10
-11
@@ -977,18 +977,19 @@ func (e *DCOMExec) getOutput() string {
|
|||||||
|
|
||||||
content, err := e.smbClient.Cat(outputPath)
|
content, err := e.smbClient.Cat(outputPath)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
e.smbClient.Rm(outputPath)
|
// Cat succeeds even while the remote process is still writing;
|
||||||
|
// Rm is what fails with a sharing violation in that case.
|
||||||
|
if rmErr := e.smbClient.Rm(outputPath); rmErr != nil && smb.IsSharingViolation(rmErr) {
|
||||||
|
waited = 0
|
||||||
|
continue
|
||||||
|
}
|
||||||
return e.decodeOutput(content)
|
return e.decodeOutput(content)
|
||||||
}
|
}
|
||||||
|
|
||||||
errStr := err.Error()
|
errStr := err.Error()
|
||||||
|
|
||||||
if strings.Contains(errStr, "STATUS_SHARING_VIOLATION") {
|
// Connection broken — reconnect and retry (matches Impacket).
|
||||||
waited = 0
|
// These come from net, not smb2, so substring matching is the right tool.
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
// Connection broken — reconnect and retry (matches Impacket)
|
|
||||||
if strings.Contains(errStr, "broken") || strings.Contains(errStr, "connection reset") || strings.Contains(errStr, "use of closed") {
|
if strings.Contains(errStr, "broken") || strings.Contains(errStr, "connection reset") || strings.Contains(errStr, "use of closed") {
|
||||||
e.log.Debug().Msg("Connection broken, trying to recreate it")
|
e.log.Debug().Msg("Connection broken, trying to recreate it")
|
||||||
e.smbClient.Close()
|
e.smbClient.Close()
|
||||||
@@ -1000,10 +1001,8 @@ func (e *DCOMExec) getOutput() string {
|
|||||||
return e.getOutput()
|
return e.getOutput()
|
||||||
}
|
}
|
||||||
|
|
||||||
// File not found yet — keep waiting up to timeout
|
// File not found yet — keep waiting up to timeout.
|
||||||
if strings.Contains(errStr, "STATUS_OBJECT_NAME_NOT_FOUND") ||
|
if smb.IsNotFound(err) {
|
||||||
strings.Contains(errStr, "does not exist") ||
|
|
||||||
strings.Contains(errStr, "not found") {
|
|
||||||
if waited >= maxWait {
|
if waited >= maxWait {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|||||||
+8
-10
@@ -379,20 +379,18 @@ func (e *SMBExec) getOutputShare() (string, error) {
|
|||||||
|
|
||||||
c, err := e.smbClient.Cat(outputFilename)
|
c, err := e.smbClient.Cat(outputFilename)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
|
// Cat succeeds even while the remote process is still writing;
|
||||||
|
// Rm is what fails with a sharing violation in that case.
|
||||||
|
if rmErr := e.smbClient.Rm(outputFilename); rmErr != nil && smb.IsSharingViolation(rmErr) {
|
||||||
|
e.log.Debug().Msg("Output file in use, waiting...")
|
||||||
|
continue
|
||||||
|
}
|
||||||
content = c
|
content = c
|
||||||
// Delete the output file
|
|
||||||
e.smbClient.Rm(outputFilename)
|
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
||||||
// If sharing violation, command is still running
|
// File not yet created — keep waiting a bit.
|
||||||
if strings.Contains(err.Error(), "STATUS_SHARING_VIOLATION") {
|
if smb.IsNotFound(err) {
|
||||||
e.log.Debug().Msg("Output file in use, waiting...")
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
// If file not found, keep waiting a bit
|
|
||||||
if strings.Contains(err.Error(), "STATUS_OBJECT_NAME_NOT_FOUND") {
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+10
-10
@@ -517,19 +517,19 @@ func (e *WMIExec) retrieveOutput(filename string) (string, error) {
|
|||||||
|
|
||||||
content, err := smbClient.Cat(filename)
|
content, err := smbClient.Cat(filename)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
// Delete the output file
|
// Cat succeeds even while the remote process is still writing
|
||||||
smbClient.Rm(filename)
|
// (read share-mode is compatible). Rm needs DELETE access, which
|
||||||
|
// is what actually conflicts — so a sharing violation here means
|
||||||
|
// the command is still running and the content is partial.
|
||||||
|
if rmErr := smbClient.Rm(filename); rmErr != nil && smb.IsSharingViolation(rmErr) {
|
||||||
|
e.log.Debug().Msg("Output file in use, waiting...")
|
||||||
|
continue
|
||||||
|
}
|
||||||
return content, nil
|
return content, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// If sharing violation, command is still running
|
// File not yet created — keep waiting.
|
||||||
if strings.Contains(err.Error(), "STATUS_SHARING_VIOLATION") {
|
if smb.IsNotFound(err) {
|
||||||
e.log.Debug().Msg("Output file in use, waiting...")
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
// If file not found, keep waiting
|
|
||||||
if strings.Contains(err.Error(), "STATUS_OBJECT_NAME_NOT_FOUND") {
|
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user