From ead6e60832a2d0ef5da220f222df896ce95c96ba Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Thu, 23 Jan 2025 16:59:30 -0800 Subject: [PATCH 1/3] Handle integer overflow cases in shellquote --- pkg/util/shellutil/shellquote.go | 49 ++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/pkg/util/shellutil/shellquote.go b/pkg/util/shellutil/shellquote.go index 1e410b8a74..4fcea9874f 100644 --- a/pkg/util/shellutil/shellquote.go +++ b/pkg/util/shellutil/shellquote.go @@ -3,7 +3,11 @@ package shellutil -import "regexp" +import ( + "log" + "math" + "regexp" +) var ( safePattern = regexp.MustCompile(`^[a-zA-Z0-9_/.-]+$`) @@ -23,10 +27,15 @@ func HardQuote(s string) string { return s } - buf := make([]byte, 0, len(s)+5) + lenS := len(s) + if lenS > math.MaxInt-5 { + log.Fatalf("string is too long to quote: %d", lenS) + } + + buf := make([]byte, 0, lenS+5) buf = append(buf, '"') - for i := 0; i < len(s); i++ { + for i := 0; i < lenS; i++ { switch s[i] { case '"', '\\', '$', '`': buf = append(buf, '\\', s[i]) @@ -51,10 +60,15 @@ func HardQuoteFish(s string) string { return s } - buf := make([]byte, 0, len(s)+5) + lenS := len(s) + if lenS > math.MaxInt-5 { + log.Fatalf("string is too long to quote: %d", lenS) + } + + buf := make([]byte, 0, lenS+5) buf = append(buf, '"') - for i := 0; i < len(s); i++ { + for i := 0; i < lenS; i++ { switch s[i] { case '"', '\\', '$': buf = append(buf, '\\', s[i]) @@ -72,10 +86,15 @@ func HardQuotePowerShell(s string) string { return "\"\"" } - buf := make([]byte, 0, len(s)+5) + lenS := len(s) + if lenS > math.MaxInt-5 { + log.Fatalf("string is too long to quote: %d", lenS) + } + + buf := make([]byte, 0, lenS+5) buf = append(buf, '"') - for i := 0; i < len(s); i++ { + for i := 0; i < lenS; i++ { c := s[i] // In PowerShell, backtick (`) is the escape character switch c { @@ -96,15 +115,17 @@ func SoftQuote(s string) string { return "\"\"" } + lenS := len(s) + // Handle special case of ~ paths - if len(s) > 0 && s[0] == '~' { + if lenS > 0 && s[0] == '~' { // If it's just ~ or ~/something with no special chars, leave it as is - if len(s) == 1 || (len(s) > 1 && s[1] == '/' && safePattern.MatchString(s[2:])) { + if lenS == 1 || (lenS > 1 && s[1] == '/' && safePattern.MatchString(s[2:])) { return s } // Otherwise quote everything after the ~ (including the /) - if len(s) > 1 && s[1] == '/' { + if lenS > 1 && s[1] == '/' { return "~" + SoftQuote(s[1:]) } } @@ -113,10 +134,14 @@ func SoftQuote(s string) string { return s } - buf := make([]byte, 0, len(s)+5) + if lenS > math.MaxInt-5 { + log.Fatalf("string is too long to quote: %d", lenS) + } + + buf := make([]byte, 0, lenS+5) buf = append(buf, '"') - for i := 0; i < len(s); i++ { + for i := 0; i < lenS; i++ { c := s[i] // In soft quote, we don't escape $ to allow expansion if c == '"' || c == '\\' || c == '`' { From df6fa5e206dd21660597a744ea164e9d9916225e Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Thu, 23 Jan 2025 17:50:51 -0800 Subject: [PATCH 2/3] cleaner handling with fixed size --- pkg/util/shellutil/shellquote.go | 55 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/pkg/util/shellutil/shellquote.go b/pkg/util/shellutil/shellquote.go index 4fcea9874f..eb25f12eb1 100644 --- a/pkg/util/shellutil/shellquote.go +++ b/pkg/util/shellutil/shellquote.go @@ -3,10 +3,10 @@ package shellutil -import ( - "log" - "math" - "regexp" +import "regexp" + +const ( + MaxQuoteSize = 10000000 // 10MB ) var ( @@ -27,15 +27,14 @@ func HardQuote(s string) string { return s } - lenS := len(s) - if lenS > math.MaxInt-5 { - log.Fatalf("string is too long to quote: %d", lenS) + if !checkQuoteSize(s) { + return "" } - buf := make([]byte, 0, lenS+5) + buf := make([]byte, 0, len(s)+5) buf = append(buf, '"') - for i := 0; i < lenS; i++ { + for i := 0; i < len(s); i++ { switch s[i] { case '"', '\\', '$', '`': buf = append(buf, '\\', s[i]) @@ -60,15 +59,14 @@ func HardQuoteFish(s string) string { return s } - lenS := len(s) - if lenS > math.MaxInt-5 { - log.Fatalf("string is too long to quote: %d", lenS) + if !checkQuoteSize(s) { + return "" } - buf := make([]byte, 0, lenS+5) + buf := make([]byte, 0, len(s)+5) buf = append(buf, '"') - for i := 0; i < lenS; i++ { + for i := 0; i < len(s); i++ { switch s[i] { case '"', '\\', '$': buf = append(buf, '\\', s[i]) @@ -86,15 +84,14 @@ func HardQuotePowerShell(s string) string { return "\"\"" } - lenS := len(s) - if lenS > math.MaxInt-5 { - log.Fatalf("string is too long to quote: %d", lenS) + if !checkQuoteSize(s) { + return "" } - buf := make([]byte, 0, lenS+5) + buf := make([]byte, 0, len(s)+5) buf = append(buf, '"') - for i := 0; i < lenS; i++ { + for i := 0; i < len(s); i++ { c := s[i] // In PowerShell, backtick (`) is the escape character switch c { @@ -115,17 +112,15 @@ func SoftQuote(s string) string { return "\"\"" } - lenS := len(s) - // Handle special case of ~ paths - if lenS > 0 && s[0] == '~' { + if len(s) > 0 && s[0] == '~' { // If it's just ~ or ~/something with no special chars, leave it as is - if lenS == 1 || (lenS > 1 && s[1] == '/' && safePattern.MatchString(s[2:])) { + if len(s) == 1 || (len(s) > 1 && s[1] == '/' && safePattern.MatchString(s[2:])) { return s } // Otherwise quote everything after the ~ (including the /) - if lenS > 1 && s[1] == '/' { + if len(s) > 1 && s[1] == '/' { return "~" + SoftQuote(s[1:]) } } @@ -134,14 +129,14 @@ func SoftQuote(s string) string { return s } - if lenS > math.MaxInt-5 { - log.Fatalf("string is too long to quote: %d", lenS) + if !checkQuoteSize(s) { + return "" } - buf := make([]byte, 0, lenS+5) + buf := make([]byte, 0, len(s)+5) buf = append(buf, '"') - for i := 0; i < lenS; i++ { + for i := 0; i < len(s); i++ { c := s[i] // In soft quote, we don't escape $ to allow expansion if c == '"' || c == '\\' || c == '`' { @@ -153,3 +148,7 @@ func SoftQuote(s string) string { buf = append(buf, '"') return string(buf) } + +func checkQuoteSize(s string) bool { + return len(s) < MaxQuoteSize +} From 560fa09941b1e25955aa127189a09b7494e99eac Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Thu, 23 Jan 2025 17:53:27 -0800 Subject: [PATCH 3/3] fix checkQuoteSize so it logs --- pkg/util/shellutil/shellquote.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/util/shellutil/shellquote.go b/pkg/util/shellutil/shellquote.go index eb25f12eb1..faa430d321 100644 --- a/pkg/util/shellutil/shellquote.go +++ b/pkg/util/shellutil/shellquote.go @@ -3,7 +3,10 @@ package shellutil -import "regexp" +import ( + "log" + "regexp" +) const ( MaxQuoteSize = 10000000 // 10MB @@ -150,5 +153,9 @@ func SoftQuote(s string) string { } func checkQuoteSize(s string) bool { - return len(s) < MaxQuoteSize + if len(s) > MaxQuoteSize { + log.Printf("string too long to quote: %s", s) + return false + } + return true }