mirror of
https://github.com/Tencent/WeKnora.git
synced 2026-06-04 13:30:32 +08:00
feat(security): implement SSRF and path traversal protections in image handling and file downloads
- Added SSRF protection by stripping client-supplied URL and Caption fields from image attachments in the QA request handler. - Introduced a validation function to ensure Feishu API path parameters contain only safe characters, preventing path traversal attacks. - Enhanced the WeCom webhook adapter to reject internal/private URLs unless they are on an allowlist, improving security during file downloads. - Implemented a mechanism to bypass SSRF checks for trusted IM platform API hosts.
This commit is contained in:
@@ -82,6 +82,14 @@ func (h *Handler) parseQARequest(c *gin.Context, logPrefix string) (*qaRequestCo
|
||||
return nil, nil, errors.NewBadRequestError("Query content cannot be empty")
|
||||
}
|
||||
|
||||
// SSRF protection: strip client-supplied URL/Caption fields from image attachments.
|
||||
// The URL field must only be populated server-side by saveImageAttachments; an
|
||||
// attacker could inject internal network URLs to trigger SSRF via the LLM provider.
|
||||
for i := range request.Images {
|
||||
request.Images[i].URL = ""
|
||||
request.Images[i].Caption = ""
|
||||
}
|
||||
|
||||
// Log request details
|
||||
if requestJSON, err := json.Marshal(request); err == nil {
|
||||
logger.Infof(ctx, "[%s] Request: session_id=%s, request=%s",
|
||||
|
||||
@@ -476,6 +476,18 @@ func (a *Adapter) SendReply(ctx context.Context, incoming *im.IncomingMessage, r
|
||||
// File download support via Feishu GetMessageResource API
|
||||
// ──────────────────────────────────────────────────────────────────────
|
||||
|
||||
// feishuSafePathParam checks that a Feishu API path parameter contains only
|
||||
// safe characters (alphanumeric, hyphen, underscore). This prevents path
|
||||
// traversal attacks via crafted callback payloads.
|
||||
func feishuSafePathParam(s string) bool {
|
||||
for _, c := range s {
|
||||
if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_') {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return len(s) > 0
|
||||
}
|
||||
|
||||
// DownloadFile downloads a file or image attachment from a Feishu message.
|
||||
// Uses the GetMessageResource API: GET /open-apis/im/v1/messages/:message_id/resources/:file_key?type={file|image}
|
||||
func (a *Adapter) DownloadFile(ctx context.Context, msg *im.IncomingMessage) (io.ReadCloser, string, error) {
|
||||
@@ -483,6 +495,11 @@ func (a *Adapter) DownloadFile(ctx context.Context, msg *im.IncomingMessage) (io
|
||||
return nil, "", fmt.Errorf("file_key and message_id are required")
|
||||
}
|
||||
|
||||
// SSRF/path-traversal protection: validate path parameters contain only safe characters
|
||||
if !feishuSafePathParam(msg.MessageID) || !feishuSafePathParam(msg.FileKey) {
|
||||
return nil, "", fmt.Errorf("invalid message_id or file_key format")
|
||||
}
|
||||
|
||||
accessToken, err := a.getTenantAccessToken(ctx)
|
||||
if err != nil {
|
||||
return nil, "", fmt.Errorf("get access token: %w", err)
|
||||
|
||||
@@ -33,6 +33,7 @@ import (
|
||||
|
||||
"github.com/Tencent/WeKnora/internal/im"
|
||||
"github.com/Tencent/WeKnora/internal/logger"
|
||||
secutils "github.com/Tencent/WeKnora/internal/utils"
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
@@ -500,6 +501,13 @@ func (a *WebhookAdapter) DownloadFile(ctx context.Context, msg *im.IncomingMessa
|
||||
// 2. Content-Type → extension mapping (fallback for platforms like WeCom that
|
||||
// don't provide the original filename in the callback JSON)
|
||||
func downloadFromURL(ctx context.Context, rawURL, fileName string) (io.ReadCloser, string, error) {
|
||||
// SSRF protection: reject internal/private URLs unless on the WeCom API allowlist.
|
||||
if !isAllowedIMAPIHost(rawURL) {
|
||||
if safe, reason := secutils.IsSSRFSafeURL(rawURL); !safe {
|
||||
return nil, "", fmt.Errorf("URL rejected for security reasons: %s", reason)
|
||||
}
|
||||
}
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)
|
||||
if err != nil {
|
||||
return nil, "", fmt.Errorf("create request: %w", err)
|
||||
@@ -573,6 +581,30 @@ func downloadFromURL(ctx context.Context, rawURL, fileName string) (io.ReadClose
|
||||
return resp.Body, fileName, nil
|
||||
}
|
||||
|
||||
// allowedIMAPIHosts lists IM platform API hosts that are trusted for file downloads.
|
||||
// URLs pointing to these hosts bypass IsSSRFSafeURL checks because the WeCom API
|
||||
// itself returns these URLs in callback payloads (e.g. temporary media links).
|
||||
var allowedIMAPIHosts = []string{
|
||||
"qyapi.weixin.qq.com",
|
||||
"api.weixin.qq.com",
|
||||
"open.work.weixin.qq.com",
|
||||
}
|
||||
|
||||
// isAllowedIMAPIHost returns true if rawURL points to a known IM platform API host.
|
||||
func isAllowedIMAPIHost(rawURL string) bool {
|
||||
u, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
hostname := strings.ToLower(u.Hostname())
|
||||
for _, allowed := range allowedIMAPIHosts {
|
||||
if hostname == allowed {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// contentTypeToExt maps common Content-Type values to file extensions.
|
||||
func contentTypeToExt(ct string) string {
|
||||
// Normalize: take only the media type, ignore parameters like charset
|
||||
|
||||
Reference in New Issue
Block a user