CodeQL Code Scanningアラート対応 (critical 1件 + high 22件のコード修正)

src/server/gkill/api/gkill_server_api/handle_browse_zip_contents.go

@@ -231,16 +231,9 @@ func extractZip(zipFilePath string, cacheDir string) error {
 		// ZIP仕様: Flags bit 11 (0x800) が立っていればUTF-8、そうでなければレガシーエンコーディング
 		entryName := decodeZipEntryName(f)
 
-		// パストラバーサル防止
-		name := filepath.FromSlash(entryName)
-		name = filepath.Clean(name)
-		if strings.HasPrefix(name, "..") || filepath.IsAbs(name) {
-			continue
-		}
-
-		destPath := filepath.Join(tmpDir, name)
-		// destPathがtmpDir配下であることを確認
-		if !strings.HasPrefix(filepath.Clean(destPath), filepath.Clean(tmpDir)+string(os.PathSeparator)) && filepath.Clean(destPath) != filepath.Clean(tmpDir) {
+		// パストラバーサル防止: tmpDir配下に収まるエントリのみ展開する
+		destPath, ok := reps.SecureJoin(tmpDir, entryName)
+		if !ok || filepath.IsAbs(filepath.FromSlash(entryName)) {
 			continue
 		}
 

src/server/gkill/api/gkill_server_api/handle_get_kyous_mcp.go

@@ -144,10 +144,8 @@ func (g *GkillServerAPI) HandleGetKyousMCP(w http.ResponseWriter, r *http.Reques
 	}
 
 	// 候補IDを収集
-	candidateCount := request.Limit
-	if candidateCount > len(batch) {
-		candidateCount = len(batch)
-	}
+	// request.Limitは冒頭でクランプ済みだが、割り当てサイズの上限を明示するためここでも定数で制限する
+	candidateCount := min(request.Limit, len(batch), maxLimit)
 	candidateIDs := make([]string, 0)
 	for i := range candidateCount {
 		candidateIDs = append(candidateIDs, batch[i].ID)

src/server/gkill/api/gkill_server_api/utils.go

@@ -14,11 +14,13 @@
 	"log/slog"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/google/uuid"
@@ -59,12 +61,11 @@
 func (g *GkillServerAPI) resolveFileName(repDir string, filename string, behavior req_res.FileUploadConflictBehavior) (string, error) {
 	// OS不正文字・制御文字を除去する (クライアント側 sanitize_filename と同じ処理)
 	filename = sanitizeFilename(filename)
-	// パストラバーサル対策: ファイル名をサニタイズしてrepDir外へのアクセスを禁止する
-	cleanFilename := filepath.Clean(filename)
-	if filepath.IsAbs(cleanFilename) || cleanFilename == ".." || strings.HasPrefix(cleanFilename, ".."+string(os.PathSeparator)) {
+	// パストラバーサル対策: repDir外へのアクセスを禁止する
+	fullFilename, ok := reps.SecureJoin(repDir, filename)
+	if !ok {
 		return "", fmt.Errorf("invalid filename: path traversal detected")
 	}
-	fullFilename := filepath.Join(repDir, cleanFilename)
 	_, err := os.Stat(fullFilename)
 	if err != nil {
 		return fullFilename, nil
@@ -306,17 +307,66 @@
 	}
 }
 
-func httpGetBase64Data(url string) (string, error) {
-	req, err := http.NewRequest("GET", url, nil)
+// maxHTTPGetBodyBytes は httpGetBase64Data が取得するレスポンスボディの上限サイズです。
+const maxHTTPGetBodyBytes = 10 * 1024 * 1024
+
+// isDisallowedFetchIP はSSRF対策として、ユーザ指定URLの取得先にできないIPか判定します。
+// loopback・プライベート・リンクローカル・マルチキャスト・未指定アドレスを拒否します。
+func isDisallowedFetchIP(ip net.IP) bool {
+	if ip == nil {
+		return true
+	}
+	return ip.IsLoopback() ||
+		ip.IsPrivate() ||
+		ip.IsUnspecified() ||
+		ip.IsLinkLocalUnicast() ||
+		ip.IsLinkLocalMulticast() ||
+		ip.IsMulticast()
+}
+
+// ssrfSafeHTTPClient はユーザ指定URLの取得に使うHTTPクライアントです。
+// Dialer.Controlで実際の接続先IPを検証するため、DNSリバインディングやリダイレクトで
+// 内部アドレスへ誘導されても接続段階で拒否されます。
+var ssrfSafeHTTPClient = &http.Client{
+	Timeout: 30 * time.Second,
+	Transport: &http.Transport{
+		DialContext: (&net.Dialer{
+			Timeout: 10 * time.Second,
+			Control: func(network, address string, c syscall.RawConn) error {
+				host, _, err := net.SplitHostPort(address)
+				if err != nil {
+					return err
+				}
+				if isDisallowedFetchIP(net.ParseIP(host)) {
+					return fmt.Errorf("blocked request to disallowed address: %s", address)
+				}
+				return nil
+			},
+		}).DialContext,
+	},
+}
+
+func httpGetBase64Data(urlString string) (string, error) {
+	parsedURL, err := url.Parse(urlString)
+	if err != nil {
+		err = fmt.Errorf("error at parse url %s: %w", urlString, err)
+		return "", err
+	}
+	if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
+		err = fmt.Errorf("unsupported url scheme %q at http get %s", parsedURL.Scheme, urlString)
+		return "", err
+	}
+
+	req, err := http.NewRequest("GET", urlString, nil)
 	if err != nil {
 		err = fmt.Errorf("error at new http get request: %w", err)
 		return "", err
 	}
-	req.Header.Set("Referer", url)
+	req.Header.Set("Referer", urlString)
 
-	res, err := http.DefaultClient.Do(req)
+	res, err := ssrfSafeHTTPClient.Do(req)
 	if err != nil {
-		err = fmt.Errorf("error at http get %s: %w", url, err)
+		err = fmt.Errorf("error at http get %s: %w", urlString, err)
 		return "", err
 	}
 	defer func() {
@@ -326,9 +376,13 @@
 		}
 	}()
 
-	b, err := io.ReadAll(res.Body)
+	b, err := io.ReadAll(io.LimitReader(res.Body, maxHTTPGetBodyBytes+1))
 	if err != nil {
-		err = fmt.Errorf("error at read all body %s: %w", url, err)
+		err = fmt.Errorf("error at read all body %s: %w", urlString, err)
+		return "", err
+	}
+	if len(b) > maxHTTPGetBodyBytes {
+		err = fmt.Errorf("response body too large at http get %s", urlString)
 		return "", err
 	}
 

src/server/gkill/api/gkill_server_api/utils_ssrf_test.go

@@ -0,0 +1,68 @@
+package gkill_server_api
+
+import (
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func TestIsDisallowedFetchIP(t *testing.T) {
+	disallowed := []string{
+		"127.0.0.1",
+		"10.0.0.1",
+		"172.16.0.1",
+		"192.168.1.1",
+		"169.254.169.254",
+		"0.0.0.0",
+		"::1",
+		"fe80::1",
+		"fc00::1",
+	}
+	for _, s := range disallowed {
+		if !isDisallowedFetchIP(net.ParseIP(s)) {
+			t.Errorf("isDisallowedFetchIP(%s) = false, want true", s)
+		}
+	}
+
+	allowed := []string{
+		"93.184.216.34",
+		"8.8.8.8",
+		"2001:4860:4860::8888",
+	}
+	for _, s := range allowed {
+		if isDisallowedFetchIP(net.ParseIP(s)) {
+			t.Errorf("isDisallowedFetchIP(%s) = true, want false", s)
+		}
+	}
+
+	if !isDisallowedFetchIP(nil) {
+		t.Error("isDisallowedFetchIP(nil) = false, want true")
+	}
+}
+
+func TestHttpGetBase64Data_BlocksLoopback(t *testing.T) {
+	// loopbackで実際にサーバを立てても、接続段階で拒否されることを確認する
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Error("request to loopback server should have been blocked")
+	}))
+	defer server.Close()
+
+	_, err := httpGetBase64Data(server.URL)
+	if err == nil {
+		t.Fatal("httpGetBase64Data to loopback should fail")
+	}
+	if !strings.Contains(err.Error(), "blocked") {
+		t.Errorf("error should mention blocked address, got: %v", err)
+	}
+}
+
+func TestHttpGetBase64Data_RejectsScheme(t *testing.T) {
+	for _, u := range []string{"file:///etc/passwd", "ftp://example.com/a", "gopher://example.com"} {
+		_, err := httpGetBase64Data(u)
+		if err == nil || !strings.Contains(err.Error(), "unsupported url scheme") {
+			t.Errorf("httpGetBase64Data(%s) should fail with scheme error, got: %v", u, err)
+		}
+	}
+}

src/server/gkill/dao/account/account_dao_sqlite3_impl.go

@@ -292,7 +292,15 @@ VALUES (
 		account.IsEnable,
 		account.PasswordResetToken,
 	}
-	slog.Log(ctx, gkill_log.TraceSQL, "sql: %s query: %#v", sql, queryArgs)
+	// パスワードハッシュ・リセットトークンはログに出さない
+	queryArgsForLog := []any{
+		account.UserID,
+		"***",
+		account.IsAdmin,
+		account.IsEnable,
+		"***",
+	}
+	slog.Log(ctx, gkill_log.TraceSQL, "sql: %s query: %#v", sql, queryArgsForLog)
 	_, err = stmt.ExecContext(ctx, queryArgs...)
 
 	if err != nil {
@@ -334,7 +342,16 @@ WHERE USER_ID = ?
 		account.PasswordResetToken,
 		account.UserID,
 	}
-	slog.Log(ctx, gkill_log.TraceSQL, "sql: %s query: %#v", sql, queryArgs)
+	// パスワードハッシュ・リセットトークンはログに出さない
+	queryArgsForLog := []any{
+		account.UserID,
+		"***",
+		account.IsAdmin,
+		account.IsEnable,
+		"***",
+		account.UserID,
+	}
+	slog.Log(ctx, gkill_log.TraceSQL, "sql: %s query: %#v", sql, queryArgsForLog)
 	_, err = stmt.ExecContext(ctx, queryArgs...)
 
 	if err != nil {

src/server/gkill/dao/plugin_manager.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/mt3hr/gkill/src/server/gkill/api/gkill_plugin"
 	"github.com/mt3hr/gkill/src/server/gkill/dao/reps"
@@ -22,8 +23,21 @@ type PluginManager struct {
 	plugins    []reps.PluginRepository
 }
 
+// isSingleSafePathElement は値を単一のパス要素として使ってよいか検証する。
+// 区切り文字・親ディレクトリ参照・空文字を含むものを拒否する。
+func isSingleSafePathElement(element string) bool {
+	if element == "" || element == "." || element == ".." {
+		return false
+	}
+	if strings.ContainsAny(element, `/\`) {
+		return false
+	}
+	return filepath.Clean(element) == element
+}
+
 // newPluginManager はユーザ別の PluginManager を生成する。
 // まだプラグインの発見は行わない。
+// userID がパス要素として不正な場合はプラグイン無しとして扱う(pluginsDirを空にする)。
 func newPluginManager(userID string) *PluginManager {
 	// GKILL_HOME は InitGkillOptions() で設定される確定済みパスを使う。
 	// gkill_options.GkillHomeDir は "$HOME/gkill" のような未展開文字列のため、
@@ -32,7 +46,12 @@ func newPluginManager(userID string) *PluginManager {
 	if pluginsBaseDir == "" || pluginsBaseDir == "$GKILL_HOME" {
 		pluginsBaseDir = filepath.Clean(os.ExpandEnv(gkill_options.GkillHomeDir))
 	}
-	pluginsDir := filepath.Join(pluginsBaseDir, "plugins", userID)
+	pluginsDir := ""
+	if isSingleSafePathElement(userID) {
+		pluginsDir = filepath.Join(pluginsBaseDir, "plugins", userID)
+	} else {
+		slog.Warn(fmt.Sprintf("invalid user id for plugin dir, plugins disabled for user %q", userID))
+	}
 	return &PluginManager{
 		userID:     userID,
 		pluginsDir: pluginsDir,
@@ -45,6 +64,10 @@ func newPluginManager(userID string) *PluginManager {
 // すでに登録済みのプラグインはスキップする（重複防止）。
 // 発見失敗は警告ログに記録し、gkill本体の起動を止めない。
 func (pm *PluginManager) DiscoverPlugins(ctx context.Context) error {
+	// pluginsDirが空 = userIDが不正でプラグイン無効
+	if pm.pluginsDir == "" {
+		return nil
+	}
 	if err := os.MkdirAll(pm.pluginsDir, os.ModePerm); err != nil {
 		// ディレクトリ作成失敗はプラグイン無しとして扱う（警告のみ）
 		slog.Warn(fmt.Sprintf("plugin dir create failed for user %s: %v", pm.userID, err))

src/server/gkill/dao/reps/idf_thumb_file_server.go

@@ -133,7 +133,7 @@ func (t *thumbFileServer) GenerateThumbCache(ctx context.Context, queryURL strin
 		return nil
 	}
 
-	abs, ok := secureJoin(t.rootDir, rel)
+	abs, ok := SecureJoin(t.rootDir, rel)
 	if !ok {
 		err := fmt.Errorf("bad path %s", queryURL)
 		return err
@@ -216,7 +216,7 @@ func (t *thumbFileServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	abs, ok := secureJoin(t.rootDir, rel)
+	abs, ok := SecureJoin(t.rootDir, rel)
 	if !ok {
 		http.Error(w, "bad path", http.StatusBadRequest)
 		return
@@ -305,8 +305,9 @@ func cleanRelURLPath(p string) (string, bool) {
 	return cp, true
 }
 
-// rootDir から外へ出ないように join
-func secureJoin(rootDir, rel string) (string, bool) {
+// SecureJoin は rootDir から外へ出ないように join する。
+// 結果が rootDir 配下でなければ ok=false を返す。
+func SecureJoin(rootDir, rel string) (string, bool) {
 	root := filepath.Clean(rootDir)
 	full := filepath.Join(root, filepath.FromSlash(rel))
 	full = filepath.Clean(full)

src/server/gkill/dao/reps/idf_video_file_server.go

@@ -82,7 +82,7 @@ func (v *IDFVideoFileServer) ensureServePathForURL(ctx context.Context, u *url.U
 		return ensuredVideo{}, false, nil
 	}
 
-	abs, ok := secureJoin(v.rootDir, rel)
+	abs, ok := SecureJoin(v.rootDir, rel)
 	if !ok {
 		return ensuredVideo{}, false, nil
 	}