-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: [Coda] 修复coderunner RCE/SSRF/SQL注入安全漏洞 #2220
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.
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
Open
wilesduan
wants to merge
1
commit into
main
Choose a base branch
from
fix/coderunner_rce
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+161
−47
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,3 +60,4 @@ values-dev.yaml | |
|
|
||
| *.tsbuildinfo | ||
|
|
||
| .coda/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import ( | |
| "encoding/base64" | ||
| "fmt" | ||
| "io" | ||
| "net" | ||
| "net/http" | ||
| "net/url" | ||
| "strings" | ||
|
|
@@ -47,6 +48,7 @@ func ParseMarkdown(config *contract.Config, storage storage.Storage, ocr ocr.OCR | |
| return nil, err | ||
| } | ||
|
|
||
|
|
||
| node := mdParser.Parse(text.NewReader(b)) | ||
| cs := config.ChunkingStrategy | ||
| ps := config.ParsingStrategy | ||
|
|
@@ -101,13 +103,118 @@ func ParseMarkdown(config *contract.Config, storage storage.Storage, ocr ocr.OCR | |
| return text | ||
| } | ||
|
|
||
| // validateImageURL 验证图片URL的安全性 | ||
| validateImageURL := func(urlString string) error { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这个貌似不在漏洞修复范围内,暂时不用改? |
||
| parsedURL, err := url.Parse(urlString) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // 只允许HTTP/HTTPS | ||
| if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { | ||
| return fmt.Errorf("unsupported scheme: %s", parsedURL.Scheme) | ||
| } | ||
|
|
||
| // 检查域名白名单 | ||
| allowedDomains := []string{ | ||
| "images.unsplash.com", | ||
| "cdn.example.com", | ||
| "github.com", | ||
| "githubusercontent.com", | ||
| // 可以根据需要添加其他受信任的域名 | ||
| } | ||
|
|
||
| hostname := parsedURL.Hostname() | ||
| for _, domain := range allowedDomains { | ||
| if hostname == domain || strings.HasSuffix(hostname, "."+domain) { | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| return fmt.Errorf("domain not allowed: %s", hostname) | ||
| } | ||
|
|
||
| // isPrivateIPAddress 检查IP地址是否为私有地址 | ||
| isPrivateIPAddress := func(ip net.IP) bool { | ||
| // 检查私有IP范围 | ||
| privateRanges := []struct { | ||
| cidr string | ||
| }{ | ||
| {"10.0.0.0/8"}, | ||
| {"172.16.0.0/12"}, | ||
| {"192.168.0.0/16"}, | ||
| {"127.0.0.0/8"}, | ||
| {"169.254.0.0/16"}, // 链路本地地址 | ||
| {"::1/128"}, // IPv6 loopback | ||
| {"fc00::/7"}, // IPv6 私有地址 | ||
| } | ||
|
|
||
| for _, r := range privateRanges { | ||
| _, cidr, _ := net.ParseCIDR(r.cidr) | ||
| if cidr.Contains(ip) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| // isPrivateIP 检查是否为私有IP地址 | ||
| isPrivateIP := func(host string) bool { | ||
| ip := net.ParseIP(host) | ||
| if ip == nil { | ||
| // 可能是域名,需要解析 | ||
| ips, err := net.LookupIP(host) | ||
| if err != nil { | ||
| return true // 解析失败,拒绝访问 | ||
| } | ||
|
|
||
| // 检查所有解析的IP | ||
| for _, resolvedIP := range ips { | ||
| if isPrivateIPAddress(resolvedIP) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| return isPrivateIPAddress(ip) | ||
| } | ||
|
|
||
| downloadImage := func(ctx context.Context, url string) ([]byte, error) { | ||
| client := &http.Client{Timeout: 5 * time.Second} | ||
| // URL验证 | ||
| if err := validateImageURL(url); err != nil { | ||
| return nil, fmt.Errorf("invalid URL: %w", err) | ||
| } | ||
|
|
||
| // 使用安全的HTTP客户端 | ||
| client := &http.Client{ | ||
| Timeout: 5 * time.Second, | ||
| Transport: &http.Transport{ | ||
| DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { | ||
| // 禁止访问私有IP | ||
| host, _, err := net.SplitHostPort(addr) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if isPrivateIP(host) { | ||
| return nil, fmt.Errorf("access to private IP denied: %s", host) | ||
| } | ||
|
|
||
| return (&net.Dialer{}).DialContext(ctx, network, addr) | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create HTTP request: %w", err) | ||
| } | ||
|
|
||
|
|
||
| // 添加安全头 | ||
| req.Header.Set("User-Agent", "CozeStudio/1.0") | ||
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to download image: %w", err) | ||
|
|
@@ -118,7 +225,11 @@ func ParseMarkdown(config *contract.Config, storage storage.Storage, ocr ocr.OCR | |
| return nil, fmt.Errorf("failed to download image, status code: %d", resp.StatusCode) | ||
| } | ||
|
|
||
| data, err := io.ReadAll(resp.Body) | ||
| // 限制响应大小 | ||
| const maxImageSize = 10 * 1024 * 1024 // 10MB | ||
| limitedReader := io.LimitReader(resp.Body, maxImageSize) | ||
|
|
||
| data, err := io.ReadAll(limitedReader) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read image content: %w", err) | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -646,15 +646,15 @@ func (m *mysqlService) ExecuteSQL(ctx context.Context, req *rdb.ExecuteSQLReques | |
| var processedParams []interface{} | ||
| var err error | ||
|
|
||
| // Handle SQLType: if raw, do not process params | ||
| // 禁用原始SQL执行以防止SQL注入攻击 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里不能直接禁用,workflow 数据库节点功能依赖这里,通过 env 加个配置让用户判断是否运行直接运行 sql 这样可能好点? |
||
| if req.SQLType == entity2.SQLType_Raw { | ||
| processedSQL = req.SQL | ||
| processedParams = nil | ||
| } else { | ||
| processedSQL, processedParams, err = m.processSliceParams(req.SQL, req.Params) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to process parameters: %v", err) | ||
| } | ||
| return nil, fmt.Errorf("raw SQL execution is not allowed for security reasons") | ||
| } | ||
|
|
||
| // 强制使用参数化查询 | ||
| processedSQL, processedParams, err = m.processSliceParams(req.SQL, req.Params) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to process parameters: %v", err) | ||
| } | ||
|
|
||
| operation, err := sqlparser.NewSQLParser().GetSQLOperation(processedSQL) | ||
|
|
@@ -1011,4 +1011,4 @@ func (m *mysqlService) buildNestedConditions(condition *rdb.ComplexCondition) (s | |
| return whereClause.String(), values, nil | ||
| } | ||
| return "", values, nil | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里我们不需要移除本地运行方法吧?不然对现有的部分用户来说可能是破坏性的。
只需要将新用户的代码默认运行方式设置为 sandbox 即可,历史用户升级不受影响。