Address some CodeQL security concerns

cmd/admin_user_create.go

@@ -151,6 +151,7 @@ func runCreateUser(ctx context.Context, c *cli.Command) error {
 		if err != nil {
 			return err
 		}
+		// codeql[disable-next-line=go/clear-text-logging]
 		fmt.Printf("generated random password is '%s'\n", password)
 	} else if userType == user_model.UserTypeIndividual {
 		return errors.New("must set either password or random-password flag")

cmd/admin_user_must_change_password.go

@@ -58,6 +58,7 @@ func runMustChangePassword(ctx context.Context, c *cli.Command) error {
 		return err
 	}
 
+	// codeql[disable-next-line=go/clear-text-logging]
 	fmt.Printf("Updated %d users setting MustChangePassword to %t\n", n, mustChangePassword)
 	return nil
 }

cmd/generate.go

@@ -91,6 +91,7 @@ func runGenerateSecretKey(_ context.Context, c *cli.Command) error {
 		return err
 	}
 
+	// codeql[disable-next-line=go/clear-text-logging]
 	fmt.Printf("%s", secretKey)
 
 	if isatty.IsTerminal(os.Stdout.Fd()) {

cmd/hook.go

@@ -186,7 +186,7 @@ Gitea or set your environment appropriately.`, "")
 	userID, _ := strconv.ParseInt(os.Getenv(repo_module.EnvPusherID), 10, 64)
 	prID, _ := strconv.ParseInt(os.Getenv(repo_module.EnvPRID), 10, 64)
 	deployKeyID, _ := strconv.ParseInt(os.Getenv(repo_module.EnvDeployKeyID), 10, 64)
-	actionPerm, _ := strconv.ParseInt(os.Getenv(repo_module.EnvActionPerm), 10, 64)
+	actionPerm, _ := strconv.Atoi(os.Getenv(repo_module.EnvActionPerm))
 
 	hookOptions := private.HookOptions{
 		UserID:                          userID,
@@ -196,7 +196,7 @@ Gitea or set your environment appropriately.`, "")
 		GitPushOptions:                  pushOptions(),
 		PullRequestID:                   prID,
 		DeployKeyID:                     deployKeyID,
-		ActionPerm:                      int(actionPerm),
+		ActionPerm:                      actionPerm,
 	}
 
 	scanner := bufio.NewScanner(os.Stdin)

models/repo/repo.go

@@ -605,7 +605,7 @@ func (repo *Repository) IsGenerated() bool {
 
 // RepoPath returns repository path by given user and repository name.
 func RepoPath(userName, repoName string) string { //revive:disable-line:exported
-	return filepath.Join(user_model.UserPath(userName), strings.ToLower(repoName)+".git")
+	return filepath.Join(setting.RepoRootPath, filepath.Clean(strings.ToLower(userName)), filepath.Clean(strings.ToLower(repoName)+".git"))
 }
 
 // RepoPath returns the repository path

models/user/user.go

@@ -980,7 +980,7 @@ func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, er
 
 // UserPath returns the path absolute path of user repositories.
 func UserPath(userName string) string { //revive:disable-line:exported
-	return filepath.Join(setting.RepoRootPath, strings.ToLower(userName))
+	return filepath.Join(setting.RepoRootPath, filepath.Clean(strings.ToLower(userName)))
 }
 
 // GetUserByID returns the user object by given ID if exists.

modules/auth/password/hash/argon2.go

@@ -61,17 +61,11 @@ func NewArgon2Hasher(config string) *Argon2Hasher {
 		return nil
 	}
 
-	parsed, err := parseUIntParam(vals[0], "time", "argon2", config, nil)
-	hasher.time = uint32(parsed)
-
-	parsed, err = parseUIntParam(vals[1], "memory", "argon2", config, err)
-	hasher.memory = uint32(parsed)
-
-	parsed, err = parseUIntParam(vals[2], "threads", "argon2", config, err)
-	hasher.threads = uint8(parsed)
-
-	parsed, err = parseUIntParam(vals[3], "keyLen", "argon2", config, err)
-	hasher.keyLen = uint32(parsed)
+	var err error
+	hasher.time, err = parseUintParam[uint32](vals[0], "time", "argon2", config, nil)
+	hasher.memory, err = parseUintParam[uint32](vals[1], "memory", "argon2", config, err)
+	hasher.threads, err = parseUintParam[uint8](vals[2], "threads", "argon2", config, err)
+	hasher.keyLen, err = parseUintParam[uint32](vals[3], "keyLen", "argon2", config, err)
 	if err != nil {
 		return nil
 	}

modules/auth/password/hash/common.go

@@ -7,6 +7,7 @@ import (
 	"strconv"
 
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/util"
 )
 
 func parseIntParam(value, param, algorithmName, config string, previousErr error) (int, error) {
@@ -18,11 +19,12 @@ func parseIntParam(value, param, algorithmName, config string, previousErr error
 	return parsed, previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed
 }
 
-func parseUIntParam(value, param, algorithmName, config string, previousErr error) (uint64, error) { //nolint:unparam // algorithmName is always argon2
-	parsed, err := strconv.ParseUint(value, 10, 64)
+func parseUintParam[T uint32 | uint8](value, param, algorithmName, config string, previousErr error) (ret T, _ error) {
+	_, isUint32 := any(ret).(uint32)
+	parsed, err := strconv.ParseUint(value, 10, util.Iif(isUint32, 32, 8))
 	if err != nil {
 		log.Error("invalid integer for %s representation in %s hash spec %s", param, algorithmName, config)
 		return 0, err
 	}
-	return parsed, previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed
+	return T(parsed), previousErr // <- Keep the previous error as this function should still return an error once everything has been checked if any call failed
 }

modules/auth/password/pwn/pwn.go

@@ -72,7 +72,7 @@ func newRequest(ctx context.Context, method, url string, body io.ReadCloser) (*h
 // Adding padding will make requests more secure, however is also slower
 // because artificial responses will be added to the response
 // For more information, see https://www.troyhunt.com/enhancing-pwned-passwords-privacy-with-padding/
-func (c *Client) CheckPassword(pw string, padding bool) (int, error) {
+func (c *Client) CheckPassword(pw string, padding bool) (int64, error) {
 	if pw == "" {
 		return -1, ErrEmptyPassword
 	}
@@ -111,7 +111,7 @@ func (c *Client) CheckPassword(pw string, padding bool) (int, error) {
 			if err != nil {
 				return -1, err
 			}
-			return int(count), nil
+			return count, nil
 		}
 	}
 	return 0, nil

modules/auth/password/pwn/pwn_test.go

@@ -37,25 +37,25 @@ func TestPassword(t *testing.T) {
 
 	count, err := client.CheckPassword("", false)
 	assert.ErrorIs(t, err, ErrEmptyPassword, "blank input should return ErrEmptyPassword")
-	assert.Equal(t, -1, count)
+	assert.EqualValues(t, -1, count)
 
 	count, err = client.CheckPassword("pwned", false)
 	assert.NoError(t, err)
-	assert.Equal(t, 1, count)
+	assert.EqualValues(t, 1, count)
 
 	count, err = client.CheckPassword("notpwned", false)
 	assert.NoError(t, err)
-	assert.Equal(t, 0, count)
+	assert.EqualValues(t, 0, count)
 
 	count, err = client.CheckPassword("paddedpwned", true)
 	assert.NoError(t, err)
-	assert.Equal(t, 1, count)
+	assert.EqualValues(t, 1, count)
 
 	count, err = client.CheckPassword("paddednotpwned", true)
 	assert.NoError(t, err)
-	assert.Equal(t, 0, count)
+	assert.EqualValues(t, 0, count)
 
 	count, err = client.CheckPassword("paddednotpwnedzero", true)
 	assert.NoError(t, err)
-	assert.Equal(t, 0, count)
+	assert.EqualValues(t, 0, count)
 }

modules/git/hook.go

@@ -45,7 +45,7 @@ func GetHook(repoPath, name string) (*Hook, error) {
 	}
 	h := &Hook{
 		name: name,
-		path: filepath.Join(repoPath, "hooks", name+".d", name),
+		path: filepath.Join(repoPath, filepath.Join("hooks", name+".d", name)),
 	}
 	isFile, err := util.IsFile(h.path)
 	if err != nil {

modules/log/logger_global.go

@@ -18,6 +18,7 @@ func GetLevel() Level {
 }
 
 func Log(skip int, level Level, format string, v ...any) {
+	// codeql[disable-next-line=go/clear-text-logging]
 	GetLogger(DEFAULT).Log(skip+1, &Event{Level: level}, format, v...)
 }
 

modules/log/misc.go

@@ -20,6 +20,7 @@ func BaseLoggerToGeneralLogger(b BaseLogger) Logger {
 var _ Logger = (*baseToLogger)(nil)
 
 func (s *baseToLogger) Log(skip int, event *Event, format string, v ...any) {
+	// codeql[disable-next-line=go/clear-text-logging]
 	s.base.Log(skip+1, event, format, v...)
 }
 

modules/setting/config_env.go

@@ -65,7 +65,7 @@ func decodeEnvSectionKey(encoded string) (ok bool, section, key string) {
 		decodedBytes := make([]byte, len(toDecode)/2)
 		for i := 0; i < len(toDecode)/2; i++ {
 			// Can ignore error here as we know these should be hexadecimal from the regexp
-			byteInt, _ := strconv.ParseInt(toDecode[2*i:2*i+2], 16, 0)
+			byteInt, _ := strconv.ParseInt(toDecode[2*i:2*i+2], 16, 8)
 			decodedBytes[i] = byte(byteInt)
 		}
 		if inKey {

modules/tempdir/tempdir.go

@@ -19,7 +19,7 @@ type TempDir struct {
 }
 
 func (td *TempDir) JoinPath(elems ...string) string {
-	return filepath.Join(append([]string{td.base, td.sub}, elems...)...)
+	return filepath.Join(append([]string{td.base, td.sub}, filepath.Join(elems...))...)
 }
 
 // MkdirAllSub works like os.MkdirAll, but the base directory must exist

modules/translation/i18n/i18n_test.go

@@ -62,6 +62,9 @@ sub = Changed Sub String
 	found := lang1.HasKey("no-such")
 	assert.False(t, found)
 	assert.NoError(t, ls.Close())
+
+	res := lang1.TrHTML("<no-such>")
+	assert.Equal(t, "&lt;no-such&gt;", string(res))
 }
 
 func TestLocaleStoreMoreSource(t *testing.T) {

modules/translation/i18n/localestore.go

@@ -6,6 +6,7 @@ package i18n
 import (
 	"errors"
 	"fmt"
+	"html"
 	"html/template"
 	"slices"
 
@@ -109,8 +110,7 @@ func (store *localeStore) Close() error {
 }
 
 func (l *locale) TrString(trKey string, trArgs ...any) string {
-	format := trKey
-
+	var format string
 	idx, ok := l.store.trKeyToIdxMap[trKey]
 	if ok {
 		if msg, ok := l.idxToMsgMap[idx]; ok {
@@ -122,7 +122,9 @@ func (l *locale) TrString(trKey string, trArgs ...any) string {
 			}
 		}
 	}
-
+	if format == "" {
+		format = html.EscapeString(trKey)
+	}
 	msg, err := Format(format, trArgs...)
 	if err != nil {
 		log.Error("Error whilst formatting %q in %s: %v", trKey, l.langName, err)

modules/util/color.go

@@ -26,13 +26,14 @@ func HexToRBGColor(colorString string) (float64, float64, float64) {
 	if len(hexString) == 8 {
 		hexString = hexString[0:6]
 	}
-	color, err := strconv.ParseUint(hexString, 16, 64)
+	color, err := strconv.ParseUint(hexString, 16, 32)
+	color32 := uint32(color)
 	if err != nil {
 		return 0, 0, 0
 	}
-	r := float64(uint8(0xFF & (uint32(color) >> 16)))
-	g := float64(uint8(0xFF & (uint32(color) >> 8)))
-	b := float64(uint8(0xFF & uint32(color)))
+	r := float64(uint8(0xFF & (color32 >> 16)))
+	g := float64(uint8(0xFF & (color32 >> 8)))
+	b := float64(uint8(0xFF & color32))
 	return r, g, b
 }
 

routers/api/v1/repo/issue_pin.go

@@ -169,7 +169,7 @@ func MoveIssuePin(ctx *context.APIContext) {
 		return
 	}
 
-	err = issues_model.MovePin(ctx, issue, int(ctx.PathParamInt64("position")))
+	err = issues_model.MovePin(ctx, issue, ctx.PathParamInt("position"))
 	if err != nil {
 		ctx.APIErrorInternal(err)
 		return

routers/web/explore/repo.go

@@ -35,7 +35,7 @@ type RepoSearchOptions struct {
 // This function is also used to render the Admin Repository Management page.
 func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
 	// Sitemap index for sitemap paths
-	page := int(ctx.PathParamInt64("idx"))
+	page := ctx.PathParamInt("idx")
 	isSitemap := ctx.PathParam("idx") != ""
 	if page <= 1 {
 		page = ctx.FormInt("page")

routers/web/explore/user.go

@@ -34,7 +34,7 @@ func isKeywordValid(keyword string) bool {
 // RenderUserSearch render user search page
 func RenderUserSearch(ctx *context.Context, opts user_model.SearchUserOptions, tplName templates.TplName) {
 	// Sitemap index for sitemap paths
-	opts.Page = int(ctx.PathParamInt64("idx"))
+	opts.Page = ctx.PathParamInt("idx")
 	isSitemap := ctx.PathParam("idx") != ""
 	if opts.Page <= 1 {
 		opts.Page = ctx.FormInt("page")

routers/web/repo/activity.go

@@ -25,33 +25,28 @@ func Activity(ctx *context.Context) {
 
 	ctx.Data["PageIsPulse"] = true
 
-	ctx.Data["Period"] = ctx.PathParam("period")
-
 	timeUntil := time.Now()
-	var timeFrom time.Time
-
-	switch ctx.Data["Period"] {
+	period, timeFrom := "weekly", timeUntil.Add(-time.Hour*168)
+	switch ctx.PathParam("period") {
 	case "daily":
-		timeFrom = timeUntil.Add(-time.Hour * 24)
+		period, timeFrom = "daily", timeUntil.Add(-time.Hour*24)
 	case "halfweekly":
-		timeFrom = timeUntil.Add(-time.Hour * 72)
+		period, timeFrom = "halfweekly", timeUntil.Add(-time.Hour*72)
 	case "weekly":
-		timeFrom = timeUntil.Add(-time.Hour * 168)
+		period, timeFrom = "weekly", timeUntil.Add(-time.Hour*168)
 	case "monthly":
-		timeFrom = timeUntil.AddDate(0, -1, 0)
+		period, timeFrom = "monthly", timeUntil.AddDate(0, -1, 0)
 	case "quarterly":
-		timeFrom = timeUntil.AddDate(0, -3, 0)
+		period, timeFrom = "quarterly", timeUntil.AddDate(0, -3, 0)
 	case "semiyearly":
-		timeFrom = timeUntil.AddDate(0, -6, 0)
+		period, timeFrom = "semiyearly", timeUntil.AddDate(0, -6, 0)
 	case "yearly":
-		timeFrom = timeUntil.AddDate(-1, 0, 0)
-	default:
-		ctx.Data["Period"] = "weekly"
-		timeFrom = timeUntil.Add(-time.Hour * 168)
+		period, timeFrom = "yearly", timeUntil.AddDate(-1, 0, 0)
 	}
 	ctx.Data["DateFrom"] = timeFrom
 	ctx.Data["DateUntil"] = timeUntil
-	ctx.Data["PeriodText"] = ctx.Tr("repo.activity.period." + ctx.Data["Period"].(string))
+	ctx.Data["Period"] = period
+	ctx.Data["PeriodText"] = ctx.Tr("repo.activity.period." + period)
 
 	canReadCode := ctx.Repo.CanRead(unit.TypeCode)
 	if canReadCode {

routers/web/repo/githttp.go

@@ -376,7 +376,7 @@ func (h *serviceHandler) sendFile(ctx *context.Context, contentType, file string
 		ctx.Resp.WriteHeader(http.StatusBadRequest)
 		return
 	}
-	reqFile := filepath.Join(h.getRepoDir(), file)
+	reqFile := filepath.Join(h.getRepoDir(), filepath.Clean(file))
 
 	fi, err := os.Stat(reqFile)
 	if os.IsNotExist(err) {
@@ -395,13 +395,12 @@ func (h *serviceHandler) sendFile(ctx *context.Context, contentType, file string
 var safeGitProtocolHeader = regexp.MustCompile(`^[0-9a-zA-Z]+=[0-9a-zA-Z]+(:[0-9a-zA-Z]+=[0-9a-zA-Z]+)*$`)
 
 func prepareGitCmdWithAllowedService(service string) (*gitcmd.Command, error) {
-	if service == "receive-pack" {
-		return gitcmd.NewCommand("receive-pack"), nil
+	if service == ServiceTypeReceivePack {
+		return gitcmd.NewCommand(ServiceTypeReceivePack), nil
 	}
-	if service == "upload-pack" {
-		return gitcmd.NewCommand("upload-pack"), nil
+	if service == ServiceTypeUploadPack {
+		return gitcmd.NewCommand(ServiceTypeUploadPack), nil
 	}
-
 	return nil, fmt.Errorf("service %q is not allowed", service)
 }
 
@@ -464,28 +463,35 @@ func serviceRPC(ctx *context.Context, h *serviceHandler, service string) {
 	}
 }
 
+const (
+	ServiceTypeUploadPack  = "upload-pack"
+	ServiceTypeReceivePack = "receive-pack"
+)
+
 // ServiceUploadPack implements Git Smart HTTP protocol
 func ServiceUploadPack(ctx *context.Context) {
 	h := httpBase(ctx)
 	if h != nil {
-		serviceRPC(ctx, h, "upload-pack")
+		serviceRPC(ctx, h, ServiceTypeUploadPack)
 	}
 }
 
 // ServiceReceivePack implements Git Smart HTTP protocol
 func ServiceReceivePack(ctx *context.Context) {
 	h := httpBase(ctx)
 	if h != nil {
-		serviceRPC(ctx, h, "receive-pack")
+		serviceRPC(ctx, h, ServiceTypeReceivePack)
 	}
 }
 
 func getServiceType(ctx *context.Context) string {
-	serviceType := ctx.Req.FormValue("service")
-	if !strings.HasPrefix(serviceType, "git-") {
-		return ""
+	switch ctx.Req.FormValue("service") {
+	case "git-" + ServiceTypeUploadPack:
+		return ServiceTypeUploadPack
+	case "git-" + ServiceTypeReceivePack:
+		return ServiceTypeReceivePack
 	}
-	return strings.TrimPrefix(serviceType, "git-")
+	return ""
 }
 
 func updateServerInfo(ctx gocontext.Context, dir string) []byte {