Skip to content

security: shell-quote interpolated values in guest command strings #61

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.

Sign up for GitHub

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

Merged
ErenAri merged 1 commit into from
Jun 28, 2026
Merged

security: shell-quote interpolated values in guest command strings #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 26 additions & 23 deletions internal/vm/qemu.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ func mapFixupArgs(fixups []MapFixup) string {
var b strings.Builder
for _, fixup := range fixups {
if fixup.MaxEntries != "" {
fmt.Fprintf(&b, " --set-map-max-entries %s=%s", fixup.Name, fixup.MaxEntries)
fmt.Fprintf(&b, " --set-map-max-entries %s", shellQuote(fmt.Sprintf("%s=%s", fixup.Name, fixup.MaxEntries)))
}
if fixup.InnerRingbufBytes > 0 {
fmt.Fprintf(&b, " --set-map-inner-ringbuf %s=%d", fixup.Name, fixup.InnerRingbufBytes)
fmt.Fprintf(&b, " --set-map-inner-ringbuf %s", shellQuote(fmt.Sprintf("%s=%d", fixup.Name, fixup.InnerRingbufBytes)))
}
if fixup.InnerMapType != "" {
fmt.Fprintf(&b, " --set-map-inner-map %s=%s:%d:%d:%d", fixup.Name,
fixup.InnerMapType, fixup.InnerKeySize, fixup.InnerValueSize, fixup.InnerMaxEntries)
fmt.Fprintf(&b, " --set-map-inner-map %s", shellQuote(fmt.Sprintf("%s=%s:%d:%d:%d", fixup.Name,
fixup.InnerMapType, fixup.InnerKeySize, fixup.InnerValueSize, fixup.InnerMaxEntries)))
}
}
return b.String()
Expand All @@ -88,25 +88,27 @@ type ProgTypeOverride struct {
func progTypeArgs(overrides []ProgTypeOverride) string {
var b strings.Builder
for _, ov := range overrides {
fmt.Fprintf(&b, " --set-prog-type %s=%s", ov.Selector, ov.Type)
fmt.Fprintf(&b, " --set-prog-type %s", shellQuote(fmt.Sprintf("%s=%s", ov.Selector, ov.Type)))
}
return b.String()
}

func progVariantArgs(groups []ProgVariantGroup) string {
var b strings.Builder
for _, group := range groups {
fmt.Fprintf(&b, " --prog-variants %s=", group.Group)
var operand strings.Builder
fmt.Fprintf(&operand, "%s=", group.Group)
for i, variant := range group.Variants {
if i > 0 {
b.WriteByte(',')
operand.WriteByte(',')
}
if variant.TrialProbe {
fmt.Fprintf(&b, "%s:trial", variant.Name)
fmt.Fprintf(&operand, "%s:trial", variant.Name)
} else {
fmt.Fprintf(&b, "%s:%d", variant.Name, variant.HelperID)
fmt.Fprintf(&operand, "%s:%d", variant.Name, variant.HelperID)
}
}
fmt.Fprintf(&b, " --prog-variants %s", shellQuote(operand.String()))
}
return b.String()
}
Expand All @@ -116,7 +118,7 @@ func progVariantArgs(groups []ProgVariantGroup) string {
func validatorTuningArgs(req ExecutionRequest) string {
args := mapFixupArgs(req.MapFixups) + progTypeArgs(req.ProgTypes) + progVariantArgs(req.ProgVariants)
if len(req.ProbeCompanions) > 0 {
args += " --probe-companions " + strings.Join(req.ProbeCompanions, ",")
args += " --probe-companions " + shellQuote(strings.Join(req.ProbeCompanions, ","))
}
return args
}
Expand Down Expand Up @@ -362,7 +364,7 @@ func ExecuteProfile(ctx context.Context, req ExecutionRequest) (result Execution
result.InfraError = err.Error()
return
}
manifestArg = fmt.Sprintf(" --manifest %s", manifestRemotePath)
manifestArg = fmt.Sprintf(" --manifest %s", shellQuote(manifestRemotePath))
}

functionalPlanArg := ""
Expand All @@ -372,7 +374,7 @@ func ExecuteProfile(ctx context.Context, req ExecutionRequest) (result Execution
result.InfraError = err.Error()
return
}
functionalPlanArg = fmt.Sprintf(" --functional-plan %s", functionalPlanRemotePath)
functionalPlanArg = fmt.Sprintf(" --functional-plan %s", shellQuote(functionalPlanRemotePath))
}

remoteResultPath := filepath.ToSlash(filepath.Join(remoteRoot, "out", "result.json"))
Expand All @@ -383,17 +385,18 @@ func ExecuteProfile(ctx context.Context, req ExecutionRequest) (result Execution
if attachMode == "" {
attachMode = "best-effort"
}
runCmd := fmt.Sprintf("sudo %s --artifact %s%s%s%s --attach-mode %s --out %s --log-dir %s/out 2>%s; code=$?; echo \"$code\" > %s; exit 0",
validatorRemotePath,
artifactRemotePath,
remoteLogDir := filepath.ToSlash(filepath.Join(remoteRoot, "out"))
runCmd := fmt.Sprintf("sudo %s --artifact %s%s%s%s --attach-mode %s --out %s --log-dir %s 2>%s; code=$?; echo \"$code\" > %s; exit 0",
shellQuote(validatorRemotePath),
shellQuote(artifactRemotePath),
manifestArg,
functionalPlanArg,
validatorTuningArgs(req),
attachMode,
remoteResultPath,
remoteRoot,
remoteErrPath,
remoteExitPath,
shellQuote(attachMode),
shellQuote(remoteResultPath),
shellQuote(remoteLogDir),
shellQuote(remoteErrPath),
shellQuote(remoteExitPath),
)
if err := sshRun(ctx, target, runCmd); err != nil {
result.InfraError = fmt.Sprintf("run validator: %v", err)
Expand Down Expand Up @@ -480,16 +483,16 @@ func guestKernelInstallCmd(release string, packageURLs []string) string {
if len(packageURLs) > 0 {
b.WriteString("mkdir -p /tmp/bpfcompat-kernel; cd /tmp/bpfcompat-kernel; ")
for i, pkg := range packageURLs {
fmt.Fprintf(&b, "curl -fsSL --retry 2 -o pkg%02d.deb '%s'; ", i, pkg)
fmt.Fprintf(&b, "curl -fsSL --retry 2 -o pkg%02d.deb %s; ", i, shellQuote(pkg))
}
b.WriteString("sudo -E dpkg -i pkg*.deb; ")
} else {
b.WriteString("sudo -E apt-get -o DPkg::Lock::Timeout=180 -q update; ")
fmt.Fprintf(&b, "sudo -E apt-get -o DPkg::Lock::Timeout=180 -q -y --no-install-recommends install linux-image-%s; ", release)
fmt.Fprintf(&b, "sudo -E apt-get -o DPkg::Lock::Timeout=180 -q -y --no-install-recommends install %s; ", shellQuote("linux-image-"+release))
}
b.WriteString("sudo sed -i 's/^GRUB_DEFAULT=.*/GRUB_DEFAULT=saved/' /etc/default/grub; ")
b.WriteString("sudo update-grub; ")
fmt.Fprintf(&b, "sudo grub-set-default 'Advanced options for Ubuntu>Ubuntu, with Linux %s'", release)
fmt.Fprintf(&b, "sudo grub-set-default %s", shellQuote("Advanced options for Ubuntu>Ubuntu, with Linux "+release))
return b.String()
}

Expand Down
33 changes: 27 additions & 6 deletions internal/vm/qemu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,9 @@ func TestMapFixupArgs(t *testing.T) {
{Name: "auxiliary_maps", MaxEntries: "cpus"},
{Name: "ringbuf_maps", MaxEntries: "16", InnerRingbufBytes: 8388608},
}
want := " --set-map-max-entries auxiliary_maps=cpus" +
" --set-map-max-entries ringbuf_maps=16" +
" --set-map-inner-ringbuf ringbuf_maps=8388608"
want := " --set-map-max-entries 'auxiliary_maps=cpus'" +
" --set-map-max-entries 'ringbuf_maps=16'" +
" --set-map-inner-ringbuf 'ringbuf_maps=8388608'"
if got := mapFixupArgs(fixups); got != want {
t.Fatalf("unexpected args:\n got %q\nwant %q", got, want)
}
Expand All @@ -434,7 +434,7 @@ func TestMapFixupArgsInnerMap(t *testing.T) {
fixups := []MapFixup{
{Name: "kubearmor_visibility", InnerMapType: "hash", InnerKeySize: 4, InnerValueSize: 4, InnerMaxEntries: 64},
}
want := " --set-map-inner-map kubearmor_visibility=hash:4:4:64"
want := " --set-map-inner-map 'kubearmor_visibility=hash:4:4:64'"
if got := mapFixupArgs(fixups); got != want {
t.Fatalf("unexpected inner-map args:\n got %q\nwant %q", got, want)
}
Expand All @@ -448,7 +448,7 @@ func TestProgVariantArgs(t *testing.T) {
{Name: "recvmmsg_old_x"},
},
}}
want := " --prog-variants recvmmsg_x=recvmmsg_x:181,recvmmsg_old_x:0"
want := " --prog-variants 'recvmmsg_x=recvmmsg_x:181,recvmmsg_old_x:0'"
if got := progVariantArgs(groups); got != want {
t.Fatalf("unexpected args:\n got %q\nwant %q", got, want)
}
Expand Down Expand Up @@ -481,8 +481,29 @@ func TestProgTypeArgs(t *testing.T) {
{Selector: "socket1", Type: "socket_filter"},
{Selector: "ig_trace_dns", Type: "socket_filter"},
})
want := " --set-prog-type socket1=socket_filter --set-prog-type ig_trace_dns=socket_filter"
want := " --set-prog-type 'socket1=socket_filter' --set-prog-type 'ig_trace_dns=socket_filter'"
if got != want {
t.Fatalf("unexpected prog-type args:\n got %q\nwant %q", got, want)
}
}

func TestShellQuote(t *testing.T) {
cases := []struct {
in string
want string
}{
{"plain", "'plain'"},
{"with space", "'with space'"},
{"name=value:1:2:3", "'name=value:1:2:3'"},
// An embedded single quote must be broken out and re-quoted so the
// value cannot terminate the surrounding quoting and inject syntax.
{"a'b", `'a'"'"'b'`},
{"$(rm -rf /)", "'$(rm -rf /)'"},
{"; reboot #", "'; reboot #'"},
}
for _, tc := range cases {
if got := shellQuote(tc.in); got != tc.want {
t.Fatalf("shellQuote(%q) = %q, want %q", tc.in, got, tc.want)
}
}
}
5 changes: 5 additions & 0 deletions internal/vm/virtme_ng.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ func shellSafeWord(value string) string {
return value
}

// shellQuote wraps value in POSIX single quotes, escaping any embedded single
// quote, so it survives intact as a single argument when an assembled command
// string is run by a remote shell (sshRun) or the in-guest validator. Manifest
// and profile values are validated upstream; quoting here is defense-in-depth
// so a validation gap can never turn an interpolated value into shell syntax.
func shellQuote(value string) string {
return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'"
}
Loading