Skip to content

Commit 7dee610

Browse files
jerrynbcjerrynbc
authored
Merge pull request #1336 from jerrynbc/fix/scan-2026-09-22
fix(management,mcp-server,yanshi): close the 2026-09-22 scan findings
2 parents c3d261b + 3afe35f commit 7dee610

16 files changed

Lines changed: 175 additions & 28 deletions

File tree

‎management/tcontrollerd/pkg/log/log.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"runtime"
77
"strings"
8+
"syscall"
89

910
"github.com/sirupsen/logrus"
1011

@@ -43,7 +44,7 @@ func InitLogger() error {
4344
return err
4445
}
4546

46-
fileFlag := os.O_WRONLY | os.O_APPEND | os.O_SYNC
47+
fileFlag := os.O_WRONLY | os.O_APPEND | os.O_SYNC | syscall.O_NOFOLLOW
4748
if !exist {
4849
if err := utils.EnsureFileDir(config.GlobalConfig.Log.Output); err != nil {
4950
return err

‎management/webserver/api/bootstrap.go‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import (
44
"crypto/rand"
55
"encoding/hex"
66
"fmt"
7+
"io"
78
"os"
89
"path/filepath"
910
"strings"
11+
"syscall"
1012
)
1113

1214
// BootstrapTokenFileEnv names the file the bootstrap token is read from when
@@ -54,10 +56,8 @@ func EnsureBootstrapToken() (created bool, path string, err error) {
5456
}
5557

5658
func loadOrCreateBootstrapToken(path string) (string, bool, error) {
57-
if content, err := os.ReadFile(path); err == nil {
58-
if token := strings.TrimSpace(string(content)); token != "" {
59-
return token, false, nil
60-
}
59+
if token, err := readBootstrapToken(path); err == nil && token != "" {
60+
return token, false, nil
6161
}
6262

6363
raw := make([]byte, bootstrapTokenBytes)
@@ -75,15 +75,16 @@ func loadOrCreateBootstrapToken(path string) (string, bool, error) {
7575
if !os.IsExist(err) {
7676
return "", false, err
7777
}
78-
content, readErr := os.ReadFile(path)
78+
token, readErr := readBootstrapToken(path)
7979
if readErr != nil {
8080
return "", false, readErr
8181
}
82-
if token := strings.TrimSpace(string(content)); token != "" {
82+
if token != "" {
8383
return token, false, nil
8484
}
85-
// Empty leftover file: replace it. Do not truncate a non-empty secret.
86-
if err := os.WriteFile(path, []byte(value+"\n"), bootstrapTokenFileMode); err != nil {
85+
// Empty leftover file: replace it without following a symlink. Do not
86+
// truncate a non-empty secret.
87+
if err := writeBootstrapTokenNoFollow(path, value); err != nil {
8788
return "", false, err
8889
}
8990
return value, true, nil
@@ -97,3 +98,36 @@ func loadOrCreateBootstrapToken(path string) (string, bool, error) {
9798
}
9899
return value, true, nil
99100
}
101+
102+
// readBootstrapToken reads the token without following a symlink. A link placed
103+
// on the shared volume must not be treated as the token file.
104+
func readBootstrapToken(path string) (string, error) {
105+
file, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW, 0)
106+
if err != nil {
107+
return "", err
108+
}
109+
defer func() {
110+
_ = file.Close()
111+
}()
112+
// Read the whole file. A prefix must not be treated as the token: the console
113+
// sends the file contents, and a truncated value would never match.
114+
content, err := io.ReadAll(file)
115+
if err != nil {
116+
return "", err
117+
}
118+
return strings.TrimSpace(string(content)), nil
119+
}
120+
121+
// writeBootstrapTokenNoFollow overwrites an existing regular file. O_NOFOLLOW
122+
// refuses a symlink instead of writing through it.
123+
func writeBootstrapTokenNoFollow(path, value string) error {
124+
file, err := os.OpenFile(path, os.O_WRONLY|os.O_TRUNC|syscall.O_NOFOLLOW, bootstrapTokenFileMode)
125+
if err != nil {
126+
return err
127+
}
128+
if _, err = file.WriteString(value + "\n"); err != nil {
129+
_ = file.Close()
130+
return err
131+
}
132+
return file.Close()
133+
}

‎management/webserver/api/cert.go‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ const (
3434
// certGenerationMaxLockout caps the wait imposed on a caller that keeps
3535
// asking.
3636
certGenerationMaxLockout = 5 * time.Minute
37+
38+
// maxUploadedCertBytes is the largest certificate or key this endpoint will
39+
// write. A PEM certificate or key is far smaller than this.
40+
maxUploadedCertBytes int64 = 1 << 20
3741
)
3842

3943
// certGenerationThrottle bounds how often this endpoint may spend a key
@@ -49,6 +53,10 @@ const (
4953
// succeeds or not.
5054
var certGenerationThrottle = newThrottle(certGenerationThreshold, certGenerationWindow, certGenerationMaxLockout)
5155

56+
// certUploadThrottle bounds how often an authenticated session may write into
57+
// the certificate directory. Generation already has a throttle; upload did not.
58+
var certUploadThrottle = newThrottle(certGenerationThreshold, certGenerationWindow, certGenerationMaxLockout)
59+
5260
// certGenerationKeys returns the throttle keys of a certificate request.
5361
func certGenerationKeys(clientIP string) []string {
5462
if clientIP == "" {
@@ -91,6 +99,21 @@ func PostUploadSSLCert(c *gin.Context) {
9199
return
92100
}
93101

102+
if file.Size <= 0 || file.Size > maxUploadedCertBytes {
103+
response.Error(c, response.JSONBody{Err: response.ErrInternalError, Msg: "Certificate file is empty or too large"}, http.StatusBadRequest)
104+
return
105+
}
106+
107+
clientIP := c.ClientIP()
108+
throttleKeys := certGenerationKeys(clientIP)
109+
if key, wait := certUploadThrottle.lockout(throttleKeys); wait > 0 {
110+
logger.Warnf("Certificate upload throttled by %s, retry in %s", key, wait)
111+
c.Header("Retry-After", strconv.FormatInt(int64(wait.Seconds())+1, 10))
112+
response.Error(c, response.JSONBody{Err: response.ErrTooManyAttempts, Msg: "Too many certificate uploads, please try again later"}, http.StatusTooManyRequests)
113+
return
114+
}
115+
certUploadThrottle.fail(throttleKeys)
116+
94117
var dstPath string
95118
filename := fmt.Sprintf("%s_%s", utils.RandStr(16), filepath.Base(file.Filename))
96119
if config.GlobalConfig.Server.DevMode {
@@ -131,7 +154,7 @@ func saveUploadedFile(file *multipart.FileHeader, dstPath string, mode os.FileMo
131154
return err
132155
}
133156

134-
if _, err = io.Copy(dst, src); err != nil {
157+
if _, err = io.Copy(dst, io.LimitReader(src, maxUploadedCertBytes)); err != nil {
135158
_ = dst.Close()
136159
return err
137160
}

‎management/webserver/api/errors.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import (
1111
// response.ErrorInternal: its text describes the inside of the deployment and
1212
// belongs in the server log, where the operator can read it.
1313
var (
14-
errDataNotExist = errors.New("Data queried does not exist")
15-
errRulesCompile = errors.New("Rules compile error, please check your params.")
14+
errDataNotExist = errors.New("Data queried does not exist")
15+
errRulesCompile = errors.New("Rules compile error, please check your params.")
16+
errPolicyRuleEmpty = errors.New("Policy rule must contain at least one pattern")
1617
)
1718

1819
// responseMessage returns the text that may be sent to the caller.
@@ -22,6 +23,8 @@ func responseMessage(err error) string {
2223
return errDataNotExist.Error()
2324
case errRulesCompile:
2425
return errRulesCompile.Error()
26+
case errPolicyRuleEmpty:
27+
return errPolicyRuleEmpty.Error()
2528
default:
2629
return response.ErrorInternal.Msg
2730
}

‎management/webserver/api/policyrule.go‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package api
22

33
import (
4+
"encoding/json"
45
"net/http"
56

67
"chaitin.cn/patronus/safeline-2/management/webserver/pkg/fvm"
78

89
"github.com/gin-gonic/gin"
10+
"gorm.io/datatypes"
911
"gorm.io/gorm"
1012

1113
"chaitin.cn/patronus/safeline-2/management/webserver/api/response"
@@ -26,6 +28,11 @@ func PostPolicyRule(ctx *gin.Context) {
2628
return
2729
}
2830

31+
if err := validatePolicyRulePatterns(params.Pattern); err != nil {
32+
response.Error(ctx, response.JSONBody{Err: response.ErrInternalError, Msg: err.Error()}, http.StatusBadRequest)
33+
return
34+
}
35+
2936
db := database.GetDB()
3037
err := db.Transaction(func(tx *gorm.DB) error {
3138
policyRule := &model.PolicyRule{Action: params.Action, Comment: params.Comment, IsEnabled: params.IsEnabled, Pattern: params.Pattern}
@@ -59,7 +66,11 @@ func PutSwitchPolicyRule(ctx *gin.Context) {
5966

6067
db := database.GetDB()
6168
err := db.Transaction(func(tx *gorm.DB) error {
62-
res := tx.Model(&model.PolicyRule{}).Where(params.IDs).Updates(model.PolicyRule{IsEnabled: params.IsEnabled})
69+
// IsEnabled=false is the Go zero value. GORM skips zero-value fields in
70+
// Updates(struct), so a disable request would leave the row enabled.
71+
res := tx.Model(&model.PolicyRule{}).Where(params.IDs).
72+
Select("is_enabled").
73+
Updates(model.PolicyRule{IsEnabled: params.IsEnabled})
6374
if res.Error != nil {
6475
return res.Error
6576
}
@@ -88,6 +99,11 @@ func PutPolicyRule(ctx *gin.Context) {
8899
return
89100
}
90101

102+
if err := validatePolicyRulePatterns(params.Pattern); err != nil {
103+
response.Error(ctx, response.JSONBody{Err: response.ErrInternalError, Msg: err.Error()}, http.StatusBadRequest)
104+
return
105+
}
106+
91107
db := database.GetDB()
92108
err := db.Transaction(func(tx *gorm.DB) error {
93109
var policyRule model.PolicyRule
@@ -153,6 +169,19 @@ func DeletePolicyRule(ctx *gin.Context) {
153169
response.Success(ctx, nil)
154170
}
155171

172+
// validatePolicyRulePatterns rejects a rule that would compile to a selector
173+
// with no WHERE. An empty pattern list matches every request.
174+
func validatePolicyRulePatterns(raw datatypes.JSON) error {
175+
if len(raw) == 0 {
176+
return errPolicyRuleEmpty
177+
}
178+
var patterns []model.PolicyRulePattern
179+
if err := json.Unmarshal(raw, &patterns); err != nil || len(patterns) == 0 {
180+
return errPolicyRuleEmpty
181+
}
182+
return nil
183+
}
184+
156185
func GetPolicyRule(ctx *gin.Context) {
157186
var params pageRequest
158187
if err := ctx.BindQuery(&params); err != nil {

‎management/webserver/api/website.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,17 @@ func PutWebsite(ctx *gin.Context) {
122122
website.CertFilename = params.CertFilename
123123
website.KeyFilename = params.KeyFilename
124124
website.IsEnabled = true
125-
tx.Save(&website)
125+
if err := tx.Save(&website).Error; err != nil {
126+
return err
127+
}
126128

127129
if config.GlobalConfig.Server.DevMode {
128130
return nil
129131
}
130132

131-
err := publishWebsiteConfig(&params)
133+
// Publish the row that was just saved, not the request body. The saved
134+
// record forces IsEnabled and may differ from params.
135+
err := publishWebsiteConfig(&website)
132136
if err != nil {
133137
return err
134138
}

‎management/webserver/pkg/fvm/fvm.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ func maybePointer(array []byte) unsafe.Pointer {
5656
}
5757

5858
func (u *FVMUpdate) ToBytes() []byte {
59+
if u == nil || u.update == nil {
60+
return nil
61+
}
5962
return C.GoBytes(unsafe.Pointer(u.update.buf.ptr), C.int(u.update.buf.length))
6063
}
6164

@@ -89,7 +92,11 @@ func (u *FVMUpdate) MergeUpdate(patch *FVMUpdate) error {
8992
}
9093

9194
func (u *FVMUpdate) Release() {
95+
if u == nil || u.update == nil {
96+
return
97+
}
9298
C.fvm_update_destroy(u.update)
99+
u.update = nil
93100
}
94101

95102
func (r *FVMRe) ToBytes() []byte {
@@ -189,6 +196,9 @@ func ReleaseOutput(out *FVMOutput) {
189196
}
190197

191198
func (f *FVM) PushFsl(server string, update *FVMUpdate) error {
199+
if update == nil || update.update == nil {
200+
return errors.New("refuse to push a nil policy update")
201+
}
192202
log.Infof("Push FSL to %s", server)
193203

194204
var length = uint32(update.update.buf.length)

‎management/webserver/pkg/fvm/generator.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,13 @@ func PolicyRuleTable(db *gorm.DB) (prFSL string, err error) {
205205
}
206206
}
207207

208+
// An empty pattern list would compile to a selector with no WHERE, which
209+
// matches every request. Skip it instead of emitting an unconditional
210+
// DROP or ACCEPT.
211+
if len(wheres) == 0 {
212+
continue
213+
}
214+
208215
// save log only when deny and dry_run
209216
logOption := "1"
210217
attackType := "-3"

‎management/webserver/pkg/fvm/helper.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ func CompileAndSave(text string) error {
2424
defer ReleaseOutput(output)
2525

2626
update := Serialize(output, false, 0, 1)
27+
if update == nil {
28+
return errors.New("failed to serialize rules")
29+
}
2730
defer update.Release()
2831

2932
// save to file. The bytecode is not secret, but nothing but the management
@@ -44,6 +47,9 @@ func CompileAndPush(text, serverAddr string) error {
4447
defer ReleaseOutput(output)
4548

4649
update := Serialize(output, false, 0, 1)
50+
if update == nil {
51+
return errors.New("failed to serialize rules")
52+
}
4753
defer update.Release()
4854

4955
// save to file. The bytecode is not secret, but nothing but the management

‎management/webserver/pkg/log/log.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"runtime"
77
"strings"
8+
"syscall"
89

910
"github.com/sirupsen/logrus"
1011

@@ -43,7 +44,9 @@ func InitLogger() error {
4344
return err
4445
}
4546

46-
fileFlag := os.O_WRONLY | os.O_APPEND | os.O_SYNC
47+
// O_NOFOLLOW closes the window between Stat and OpenFile: a symlink swapped
48+
// in for the log path is refused instead of followed.
49+
fileFlag := os.O_WRONLY | os.O_APPEND | os.O_SYNC | syscall.O_NOFOLLOW
4750
if !exist {
4851
if err := utils.EnsureFileDir(config.GlobalConfig.Log.Output); err != nil {
4952
return err

0 commit comments

Comments
 (0)