Extensionargs

[C-Sto]

Card

Ace of Spades

Details

Adjusts DLL extensions to use the same argument API as COFF/BOF extensions. This will rely on #1397 (and is based on it), but the changes to the client are pretty minimal, so could be rebased fairly easily. Check the diff from 4421ec1 to see the actual changes. Alternatively, close this PR and I can add the argument changes to #1397 to keep it all together (but I figured that an API change like this would probably warrant some public discussion).

Would like feedback on how to handle 'legacy' extensions that use a single argument - currently any extension that takes a single string argument per the manifest is in 'legacy' mode, and the string does not have a length prefix in the data stream. This might complicate things if people are developing extensions and not looking at the docs that should shout about making sure you do/don't expect length prefixes. Alternatively, we can scrap the legacy mode and just force usage of the BOF arg packer.

It may also be worth publishing a sliverextensionutils package somewhere that has the equivalent of the beacon.h headers to make extension development less tedious. I've included the slivercompat.go I used to verify this PR below. This might be a good opportunity to make the callback routines a bit more mature as well (eg, including the ability to send data back as a download/loot and/or just generally more asynclyfied).

slivercompat.go

package main

import (
	"encoding/binary"
	"fmt"
	"strings"
	"syscall"
	"unsafe"
)

type dataParser struct {
	original []byte
	n        int
}

// args should work similar to how bofs work - so we will present a familiar interface for getting args
func beaconDataParse(data, dataLen uintptr) (*dataParser, error) {
	if data == 0 || dataLen == 0 {
		return nil, fmt.Errorf("no data to parse")
	}
	//turn uintptrs into slices
	dp := dataParser{
		original: unsafe.Slice((*byte)(unsafe.Pointer(data)), dataLen),
		n:        4,
	}
	return &dp, nil
}

// BeaconDataExtract returns a slice of bytes. The underlying data could be a string, a file, etc.
func (dp *dataParser) BeaconDataExtract() ([]byte, error) {
	if dp.BeaconDataLength() < 4 {
		return nil, fmt.Errorf("no more data to return")
	}
	//extract the length
	l := binary.LittleEndian.Uint32(dp.original[dp.n:])
	//increment n
	dp.n += 4
	//copy to a new buffer to avoid mutating the underlying state
	if dp.BeaconDataLength() < int(l) {
		return nil, fmt.Errorf("no more data to return")
	}
	rb := make([]byte, l)
	c := copy(rb, dp.original[dp.n:])
	if c != int(l) {
		return nil, fmt.Errorf("expected to copy %x but copied %x", l, c)
	}
	//increment n
	dp.n += c

	return rb, nil
}

// BeaconDataInt returns a uint32 from the data parser
func (dp *dataParser) BeaconDataInt() (uint32, error) {
	if dp.BeaconDataLength() < 4 {
		return 0, fmt.Errorf("no more data to return")
	}
	r := binary.LittleEndian.Uint32(dp.original[dp.n:])
	dp.n += 4
	return r, nil
}

// BeaconDataShort returns a uint16 from the data parser
func (dp *dataParser) BeaconDataShort() (uint16, error) {
	if dp.BeaconDataLength() < 2 {
		return 0, fmt.Errorf("no more data to return")
	}
	r := binary.LittleEndian.Uint16(dp.original[dp.n:])
	dp.n += 2
	return r, nil
}

// BeaconDataLength returns the remaining length of unparsed data
func (dp *dataParser) BeaconDataLength() int {
	return len(dp.original) - dp.n
}

type outputBuffer struct {
	b        strings.Builder
	done     bool
	callback uintptr
}

func newOutBuffer(callback uintptr) *outputBuffer {
	return &outputBuffer{b: strings.Builder{}, callback: callback}
}

func (o *outputBuffer) sendOutput(data string) {
	//errors not captured here, where would we send them?
	o.b.WriteString(data)
	o.b.WriteString("\n") //newline
}

func (o *outputBuffer) sendError(err error) {
	o.sendOutput(fmt.Sprintf("error: %s", err.Error()))
}

func (o *outputBuffer) flush() {
	//write the buffer - checks if it's already been flushed to avoid crashing the process
	if o.done {
		//figure out a way to return a message to implant that something went wrong with flush
		return
	}
	_sendOutput(o.b.String(), o.callback)
	o.done = true
}

// data should only be sent once per call from implant (for now)
func _sendOutput(data string, callback uintptr) {
	outDataPtr, err := syscall.BytePtrFromString(data)
	if err != nil {
		return
	}
	// Send data back
	syscall.SyscallN(callback, uintptr(unsafe.Pointer(outDataPtr)), uintptr(len(data)))
}

usage might look like below:

exampleext.go

//export ArgsStr
func ArgsStr(data uintptr, dataLen uintptr, callback uintptr) uintptr {
	//confirming that legacy argument mode works (single string argument)
	ob := newOutBuffer(callback)
	defer ob.flush() //should call flush at function end

	datab := unsafe.Slice((*byte)(unsafe.Pointer(data)), dataLen)

	ob.sendOutput(string("got data: " + string(datab)))
	return 0
}

//export ArgsTest
func ArgsTest(data uintptr, dataLen uintptr, callback uintptr) uintptr {
	ob := newOutBuffer(callback)
	defer ob.flush() //should call flush at function end

	//string, file, int, wstring, short
	dp, err := beaconDataParse(data, dataLen)
	if err != nil {
		ob.sendError(err)
		return 2
	}

	strArg1_b, err := dp.BeaconDataExtract()
	if err != nil {
		ob.sendError(err)
		return 2
	}
	//strings are zero terminated and not stripped by the extract function, so we remove the trailing nulls here
	strArg1 := string(strArg1_b)[:len(strArg1_b)-1]
	ob.sendOutput(fmt.Sprintf("string (arg1): %s %x (%d bytes remaining in parser)", strArg1, strArg1, dp.BeaconDataLength()))
	fileArg1, err := dp.BeaconDataExtract()
	if err != nil {
		ob.sendError(err)
		return 2
	}
	printLen := 10
	if len(fileArg1) < printLen {
		printLen = len(fileArg1) - 1
	}

	ob.sendOutput(fmt.Sprintf("first 10 bytes of file (arg2): %x (full length %d , %d bytes remaining in parser)", fileArg1[:printLen], printLen, dp.BeaconDataLength()))

	intArg, err := dp.BeaconDataInt()
	if err != nil {
		ob.sendError(err)
		return 2
	}
	ob.sendOutput(fmt.Sprintf("int (arg3): %d (%d bytes remaining in parser)", intArg, dp.BeaconDataLength()))

	strArg2_b, err := dp.BeaconDataExtract()
	if err != nil {
		ob.sendError(err)
		return 2
	}
	//sigh, unicode
	decoder := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM).NewDecoder()
	bStrData, err := decoder.Bytes(strArg2_b[:len(strArg2_b)-2]) //strip trailing nulls before decoding
	if err != nil {
		ob.sendError(err)
		return 2
	}
	strData := string(bStrData)
	ob.sendOutput(fmt.Sprintf("string (arg4): %s %x (%d bytes remaining in parser)", strData, strData, dp.BeaconDataLength()))

	shortArg, err := dp.BeaconDataShort()
	if err != nil {
		ob.sendError(err)
		return 2
	}
	ob.sendOutput(fmt.Sprintf("short (arg5): %d (%d bytes remaining in parser)", shortArg, dp.BeaconDataLength()))

	return 0
}

extension.json

{
    "name": "example-multientry-argbois",
    "version": "0.0.0",
    "extension_author": "c_sto",
    "original_author": "c_sto",
    "repo_url": "no",
    "commands": [
        {
            "command_name": "ArgsStr",
            "help": "argsstr",
            "entrypoint": "ArgsStr",
            "files": [
                {
                    "os": "windows",
                    "arch": "amd64",
                    "path": "ex.dll"
                }
            ],
            "arguments": [
                {
                    "name": "string1",
                    "desc": "string1",
                    "type": "string",
                    "optional": false
                }
            ]
        },
        {
            "command_name": "ArgsTest",
            "help": "startw",
            "entrypoint": "ArgsTest",
            "files": [
                {
                    "os": "windows",
                    "arch": "amd64",
                    "path": "ex.dll"
                }
            ],
            "arguments": [
                {
                    "name": "string1",
                    "desc": "string1",
                    "type": "string",
                    "optional": false
                },
                {
                    "name": "file1",
                    "desc": "file1",
                    "type": "file"
                },
                {
                    "name": "int1",
                    "desc": "int1",
                    "type": "int"
                },
                {
                    "name": "string2",
                    "desc": "int1",
                    "type": "wstring"
                },
                {
                    "name": "short1",
                    "desc": "short1",
                    "type": "short"
                }
            ]
        }
    ]
}

[rkervella]

Would like feedback on how to handle 'legacy' extensions that use a single argument - currently any extension that takes a single string argument per the manifest is in 'legacy' mode, and the string does not have a length prefix in the data stream.

I can see two solutions:

• compatibility mode until next big release that way we don't have to modify anything and "it all just works"
• drop support now and adapt all the extensions in the public armory (it shouldn't affect a lot of those)

I'm in favor of the second option sine we'll have to do it eventually. We could put a warning in the UI when loading the extensions and seeing the manifest doesn't contain what we expect, and link to the docs so people can update.

It may also be worth publishing a sliverextensionutils package somewhere that has the equivalent of the beacon.h headers to make extension development less tedious.

I'm working on a side project that is basically bootstrap templates for extensions. This will solve that issue, as all people would need to do is clone a repo and put their code in the templated files. Argument parsers will be in there.

I'll try to review as soon as I can, I've been busy this year :)

[rkervella]

Some conflicts to resolve before I can test this one (since I merged the other PR).

[C-Sto]

Should be good now - reminder that

sliver/client/command/extensions/load.go

Line 493 in dfaadfb

if len(ext.Arguments) == 1 && ext.Arguments[0].Type == "string" {

means that extensions with a single string argument that are not bofs won't be using the argument unpacking/packing logic. Or to rephrase: all dll extensions must use the bof argument packing/unpacking logic, unless they only have a single string argument.

This can be changed by adjusting the check in the code ref'd above to just apply the bof arg packing logic to all extensions (including those that only take a single string), but it will break old extensions that expect a 'dumb' string.

[rkervella]

Should be good now - reminder that

sliver/client/command/extensions/load.go

Line 493 in dfaadfb

if len(ext.Arguments) == 1 && ext.Arguments[0].Type == "string" {

means that extensions with a single string argument that are not bofs won't be using the argument unpacking/packing logic. Or to rephrase: all dll extensions must use the bof argument packing/unpacking logic, unless they only have a single string argument.
This can be changed by adjusting the check in the code ref'd above to just apply the bof arg packing logic to all extensions (including those that only take a single string), but it will break old extensions that expect a 'dumb' string.

We'll make sure to document that, otherwise I'm sure I'll forget about it.

Edit: actually I think we can break things here and force the usage of the BOF argument packing/unpacking even for single string arguments. There's only two or three DLL extensions in the armory anyway, and I haven't seen a ton of non-bof extensions out there. Once I'm done with the Sliver SDK, developers will be able to include the parser code in their code base and be done with it.