Skip to content

Commit

Permalink
Merge pull request from GHSA-wx8q-4gm9-rj2g
Browse files Browse the repository at this point in the history
* Fix JuicefsRuntime: escape customized string before constructing commands

add escapeBashStr

Signed-off-by: xixi <hexilee@juicedata.io>

avoid bash -c in operations

Signed-off-by: xixi <hexilee@juicedata.io>

fix GetUsedSpace and GetFileCount

Signed-off-by: xixi <hexilee@juicedata.io>

move EscapeBashStr to pkg/utils/security

Signed-off-by: xixi <hexilee@juicedata.io>

add left

Signed-off-by: xixi <hexilee@juicedata.io>

resume GetFileCount

Signed-off-by: xixi <hexilee@juicedata.io>

Escape value.Configs.Name

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

Fix unit tests

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

Upgrade juicefs helm chart version to 0.2.16

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Fix JuicefsRuntime: escape customized string before constructing commands

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

---------

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Co-authored-by: xixi <hexilee@juicedata.io>
  • Loading branch information
TrafalgarZZZ and Hexilee authored Mar 13, 2024
1 parent 8637168 commit e0184cf
Show file tree
Hide file tree
Showing 7 changed files with 166 additions and 32 deletions.
2 changes: 1 addition & 1 deletion charts/juicefs/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: juicefs
apiVersion: v1
description: FileSystem aimed for data analytics and machine learning in any cloud.
version: 0.2.14
version: 0.2.16
appVersion: v1.0.0
home: https://juicefs.com/
maintainers:
Expand Down
23 changes: 14 additions & 9 deletions pkg/ddc/juicefs/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/go-logr/logr"

"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/fluid-cloudnative/fluid/pkg/utils/security"
)

type JuiceFileUtils struct {
Expand Down Expand Up @@ -130,12 +131,11 @@ func (j JuiceFileUtils) Count(juiceSubPath string) (total int64, err error) {
func (j JuiceFileUtils) GetFileCount(juiceSubPath string) (fileCount int64, err error) {
var (
//strs = "du -ah juiceSubPath |grep ^- |wc -l "
strs = fmt.Sprintf("ls -lR %s |grep ^- |wc -l ", juiceSubPath)
strs = fmt.Sprintf("ls -lR %s |grep ^- |wc -l ", security.EscapeBashStr(juiceSubPath))
command = []string{"bash", "-c", strs}
stdout string
stderr string
)

stdout, stderr, err = j.exec(command)
if err != nil {
err = fmt.Errorf("execute command %v with expectedErr: %v stdout %s and stderr %s", command, err, stdout, stderr)
Expand Down Expand Up @@ -266,11 +266,10 @@ func (j JuiceFileUtils) GetMetric(juicefsPath string) (metrics string, err error
}

// GetUsedSpace Get used space in byte
// use "df --block-size=1 |grep <juicefsPath>'"
// equal to `df --block-size=1 | grep juicefsPath`
func (j JuiceFileUtils) GetUsedSpace(juicefsPath string) (usedSpace int64, err error) {
var (
strs = fmt.Sprintf(`df --block-size=1 |grep %s`, juicefsPath)
command = []string{"bash", "-c", strs}
command = []string{"df", "--block-size=1"}
stdout string
stderr string
)
Expand All @@ -281,9 +280,15 @@ func (j JuiceFileUtils) GetUsedSpace(juicefsPath string) (usedSpace int64, err e
return
}

var str string
lines := strings.Split(stdout, "\n")
for _, line := range lines {
if strings.Contains(line, juicefsPath) {
str = line
break
}
}
// [<Filesystem> <Size> <Used> <Avail> <Use>% <Mounted on>]
str := strings.TrimSuffix(stdout, "\n")

data := strings.Fields(str)
if len(data) != 6 {
err = fmt.Errorf("failed to parse %s in GetUsedSpace method", data)
Expand Down Expand Up @@ -365,8 +370,8 @@ func (j JuiceFileUtils) QueryMetaDataInfoIntoFile(key KeyOfMetaDataFile, filenam
j.log.Error(errors.New("the key not in metadatafile"), "key", key)
}
var (
str = "sed -n '" + line + "' " + filename
command = []string{"bash", "-c", str}
str = "'" + line + "' " + filename
command = []string{"sed", "-n", str}
stdout string
stderr string
)
Expand Down
4 changes: 2 additions & 2 deletions pkg/ddc/juicefs/operations/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ func TestJuiceFileUtils_GetUsedSpace(t *testing.T) {
t.Fatal(err.Error())
}
a := &JuiceFileUtils{log: fake.NullLogger()}
_, err = a.GetUsedSpace("/tmp")
_, err = a.GetUsedSpace("/runtime-mnt/juicefs/kube-system/jfsdemo/juicefs-fuse")
if err == nil {
t.Error("check failure, want err, got nil")
}
Expand All @@ -489,7 +489,7 @@ func TestJuiceFileUtils_GetUsedSpace(t *testing.T) {
if err != nil {
t.Fatal(err.Error())
}
usedSpace, err := a.GetUsedSpace("/tmp")
usedSpace, err := a.GetUsedSpace("/runtime-mnt/juicefs/kube-system/jfsdemo/juicefs-fuse")
if err != nil {
t.Errorf("check failure, want nil, got err: %v", err)
}
Expand Down
55 changes: 40 additions & 15 deletions pkg/ddc/juicefs/transform_fuse.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/fluid-cloudnative/fluid/pkg/common"
"github.com/fluid-cloudnative/fluid/pkg/utils"
"github.com/fluid-cloudnative/fluid/pkg/utils/kubeclient"
"github.com/fluid-cloudnative/fluid/pkg/utils/security"
)

func (j *JuiceFSEngine) transformFuse(runtime *datav1alpha1.JuiceFSRuntime, dataset *datav1alpha1.Dataset, value *JuiceFS) (err error) {
Expand All @@ -37,7 +38,7 @@ func (j *JuiceFSEngine) transformFuse(runtime *datav1alpha1.JuiceFSRuntime, data
}
mount := dataset.Spec.Mounts[0]

value.Configs.Name = mount.Name
value.Configs.Name = security.EscapeBashStr(mount.Name)

// transform image
image := runtime.Spec.Fuse.Image
Expand Down Expand Up @@ -216,7 +217,7 @@ func (j *JuiceFSEngine) genValue(mount datav1alpha1.Mount, tiredStoreLevel *data
}

if source == "" {
source = mount.Name
source = security.EscapeBashStr(mount.Name)
}

// transform source
Expand Down Expand Up @@ -326,8 +327,20 @@ func (j *JuiceFSEngine) genMount(value *JuiceFS, runtime *datav1alpha1.JuiceFSRu
}
workerOptionMap["metrics"] = fmt.Sprintf("0.0.0.0:%d", metricsPort)
}
mountArgs = []string{common.JuiceFSCeMountPath, value.Source, value.Fuse.MountPath, "-o", strings.Join(genOption(optionMap), ",")}
mountArgsWorker = []string{common.JuiceFSCeMountPath, value.Source, value.Worker.MountPath, "-o", strings.Join(genOption(workerOptionMap), ",")}
mountArgs = []string{
common.JuiceFSCeMountPath,
value.Source,
security.EscapeBashStr(value.Fuse.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genOption(optionMap), ",")),
}
mountArgsWorker = []string{
common.JuiceFSCeMountPath,
value.Source,
security.EscapeBashStr(value.Worker.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genOption(workerOptionMap), ",")),
}
} else {
if readonly {
optionMap["attrcacheto"] = "7200"
Expand All @@ -347,14 +360,26 @@ func (j *JuiceFSEngine) genMount(value *JuiceFS, runtime *datav1alpha1.JuiceFSRu
optionMap["no-sharing"] = ""
delete(workerOptionMap, "no-sharing")

mountArgs = []string{common.JuiceFSMountPath, value.Source, value.Fuse.MountPath, "-o", strings.Join(genOption(optionMap), ",")}
mountArgsWorker = []string{common.JuiceFSMountPath, value.Source, value.Worker.MountPath, "-o", strings.Join(genOption(workerOptionMap), ",")}
mountArgs = []string{
common.JuiceFSMountPath,
value.Source,
security.EscapeBashStr(value.Fuse.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genOption(optionMap), ",")),
}
mountArgsWorker = []string{
common.JuiceFSMountPath,
value.Source,
security.EscapeBashStr(value.Worker.MountPath),
"-o",
security.EscapeBashStr(strings.Join(genOption(workerOptionMap), ",")),
}
}

value.Worker.Command = strings.Join(mountArgsWorker, " ")
value.Fuse.Command = strings.Join(mountArgs, " ")
value.Fuse.StatCmd = "stat -c %i " + value.Fuse.MountPath
value.Worker.StatCmd = "stat -c %i " + value.Worker.MountPath
value.Fuse.StatCmd = "stat -c %i " + security.EscapeBashStr(value.Fuse.MountPath)
value.Worker.StatCmd = "stat -c %i " + security.EscapeBashStr(value.Worker.MountPath)
return nil
}

Expand All @@ -379,7 +404,7 @@ func (j *JuiceFSEngine) genFormatCmd(value *JuiceFS, config *[]string) {
for _, option := range *config {
o := strings.TrimSpace(option)
if o != "" {
args = append(args, fmt.Sprintf("--%s", o))
args = append(args, fmt.Sprintf("--%s", security.EscapeBashStr(o)))
}
}
}
Expand All @@ -395,12 +420,12 @@ func (j *JuiceFSEngine) genFormatCmd(value *JuiceFS, config *[]string) {
args = append(args, "--no-update")
}
if value.Configs.Storage != "" {
args = append(args, fmt.Sprintf("--storage=%s", value.Configs.Storage))
args = append(args, fmt.Sprintf("--storage=%s", security.EscapeBashStr(value.Configs.Storage)))
}
if value.Configs.Bucket != "" {
args = append(args, fmt.Sprintf("--bucket=%s", value.Configs.Bucket))
args = append(args, fmt.Sprintf("--bucket=%s", security.EscapeBashStr(value.Configs.Bucket)))
}
args = append(args, value.Source, value.Configs.Name)
args = append(args, value.Source, security.EscapeBashStr(value.Configs.Name))
cmd := append([]string{common.JuiceCeCliPath, "format"}, args...)
value.Configs.FormatCmd = strings.Join(cmd, " ")
return
Expand All @@ -418,7 +443,7 @@ func (j *JuiceFSEngine) genFormatCmd(value *JuiceFS, config *[]string) {
args = append(args, "--secretkey=${SECRET_KEY}")
}
if value.Configs.Bucket != "" {
args = append(args, fmt.Sprintf("--bucket=%s", value.Configs.Bucket))
args = append(args, fmt.Sprintf("--bucket=%s", security.EscapeBashStr(value.Configs.Bucket)))
}
args = append(args, value.Source)
cmd := append([]string{common.JuiceCliPath, "auth"}, args...)
Expand Down Expand Up @@ -461,7 +486,7 @@ func (j *JuiceFSEngine) genQuotaCmd(value *JuiceFS, mount datav1alpha1.Mount) er
return fmt.Errorf("quota is not supported in juicefs-ce version %s", value.Fuse.ImageTag)
}
// juicefs quota set ${metaurl} --path ${path} --capacity ${capacity}
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", common.JuiceCeCliPath, value.Source, value.Fuse.SubPath, qs)
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", common.JuiceCeCliPath, value.Source, security.EscapeBashStr(value.Fuse.SubPath), qs)
return nil
}
// ee
Expand All @@ -470,7 +495,7 @@ func (j *JuiceFSEngine) genQuotaCmd(value *JuiceFS, mount datav1alpha1.Mount) er
}
// juicefs quota set ${metaurl} --path ${path} --capacity ${capacity}
cli := common.JuiceCliPath
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", cli, value.Source, value.Fuse.SubPath, qs)
value.Configs.QuotaCmd = fmt.Sprintf("%s quota set %s --path %s --capacity %d", cli, value.Source, security.EscapeBashStr(value.Fuse.SubPath), qs)
return nil
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/ddc/juicefs/ufs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ func mockExecCommandInContainerForTotalFileNums() (stdout string, stderr string,
}

func mockExecCommandInContainerForUsedStorageBytes() (stdout string, stderr string, err error) {
r := `JuiceFS:test 207300683100160 41460043776 207259223056384 1% /data`
r := `JuiceFS:test 207300683100160 41460043776 207259223056384 1% /juicefs/juicefs/test/juicefs-fuse`
return r, "", nil
}

func TestTotalStorageBytes(t *testing.T) {
statefulSet := &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-worker",
Namespace: "fluid",
Namespace: "juicefs",
},
Spec: appsv1.StatefulSetSpec{
Selector: &metav1.LabelSelector{
Expand All @@ -57,7 +57,7 @@ func TestTotalStorageBytes(t *testing.T) {
var pod = &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test-work-0",
Namespace: "fluid",
Namespace: "juicefs",
Labels: map[string]string{"a": "b"},
},
Status: corev1.PodStatus{
Expand Down Expand Up @@ -93,11 +93,11 @@ func TestTotalStorageBytes(t *testing.T) {
name: "test",
fields: fields{
name: "test",
namespace: "fluid",
namespace: "juicefs",
runtime: &datav1alpha1.JuiceFSRuntime{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
Namespace: "fluid",
Namespace: "juicefs",
},
},
},
Expand Down
63 changes: 63 additions & 0 deletions pkg/utils/security/escape.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
Copyright 2023 The Fluid Authors.
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 security

import (
"fmt"
"strings"
)

// According to https://www.gnu.org/software/bash/manual/html_node/ANSI_002dC-Quoting.html#ANSI_002dC-Quoting
// a -> a
// a b -> a b
// $a -> $'$a'
// $'a' -> $'$\'$a'\'
func EscapeBashStr(s string) string {
if !containsOne(s, []rune{'$', '`', '&', ';', '>', '|', '(', ')'}) {
return s
}
s = strings.ReplaceAll(s, `\`, `\\`)
s = strings.ReplaceAll(s, `'`, `\'`)
if strings.Contains(s, `\\`) {
s = strings.ReplaceAll(s, `\\\\`, `\\`)
s = strings.ReplaceAll(s, `\\\'`, `\'`)
s = strings.ReplaceAll(s, `\\"`, `\"`)
s = strings.ReplaceAll(s, `\\a`, `\a`)
s = strings.ReplaceAll(s, `\\b`, `\b`)
s = strings.ReplaceAll(s, `\\e`, `\e`)
s = strings.ReplaceAll(s, `\\E`, `\E`)
s = strings.ReplaceAll(s, `\\n`, `\n`)
s = strings.ReplaceAll(s, `\\r`, `\r`)
s = strings.ReplaceAll(s, `\\t`, `\t`)
s = strings.ReplaceAll(s, `\\v`, `\v`)
s = strings.ReplaceAll(s, `\\?`, `\?`)
}
return fmt.Sprintf(`$'%s'`, s)
}

func containsOne(target string, chars []rune) bool {
charMap := make(map[rune]bool, len(chars))
for _, c := range chars {
charMap[c] = true
}
for _, s := range target {
if charMap[s] {
return true
}
}
return false
}
41 changes: 41 additions & 0 deletions pkg/utils/security/escape_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
Copyright 2023 The Fluid Author.
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 security

import "testing"

func TestEscapeBashStr(t *testing.T) {
cases := [][]string{
{"abc", "abc"},
{"test-volume", "test-volume"},
{"http://minio.kube-system:9000/minio/dynamic-ce", "http://minio.kube-system:9000/minio/dynamic-ce"},
{"$(cat /proc/self/status | grep CapEff > /test.txt)", "$'$(cat /proc/self/status | grep CapEff > /test.txt)'"},
{"hel`cat /proc/self/status`lo", "$'hel`cat /proc/self/status`lo'"},
{"'h'el`cat /proc/self/status`lo", "$'\\'h\\'el`cat /proc/self/status`lo'"},
{"\\'h\\'el`cat /proc/self/status`lo", "$'\\'h\\'el`cat /proc/self/status`lo'"},
{"$'h'el`cat /proc/self/status`lo", "$'$\\'h\\'el`cat /proc/self/status`lo'"},
{"hel\\`cat /proc/self/status`lo", "$'hel\\\\`cat /proc/self/status`lo'"},
{"hel\\\\`cat /proc/self/status`lo", "$'hel\\\\`cat /proc/self/status`lo'"},
{"hel\\'`cat /proc/self/status`lo", "$'hel\\'`cat /proc/self/status`lo'"},
}
for _, c := range cases {
escaped := EscapeBashStr(c[0])
if escaped != c[1] {
t.Errorf("escapeBashVar(%s) = %s, want %s", c[0], escaped, c[1])
}
}
}

0 comments on commit e0184cf

Please sign in to comment.