Skip to content

Commit 0da4e53

Browse files
suzp1984suzp1984
committed
fix code scanning warning: uncontrolled data used in path expression.
1 parent d1ffc2a commit 0da4e53

File tree

6 files changed

+88
-155
lines changed

6 files changed

+88
-155
lines changed

‎platform/dvr-local-disk.go

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package main
55

66
import (
7+
"bytes"
78
"context"
89
"encoding/json"
910
"fmt"
@@ -535,21 +536,16 @@ func (v *RecordWorker) Handle(ctx context.Context, handler *http.ServeMux) error
535536
return nil
536537
}
537538

538-
func (v *RecordWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) error {
539+
func (v *RecordWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage, data []byte) error {
539540
// Copy the ts file to temporary cache dir.
540541
tsid := uuid.NewString()
541542
tsfile := path.Join("record", fmt.Sprintf("%v.ts", tsid))
542543

543-
// Always use execFile when params contains user inputs, see https://auth0.com/blog/preventing-command-injection-attacks-in-node-js-apps/
544-
// Note that should never use fs.copyFileSync(file, tsfile, fs.constants.COPYFILE_FICLONE_FORCE) which fails in macOS.
545-
if err := exec.CommandContext(ctx, "cp", "-f", msg.File, tsfile).Run(); err != nil {
546-
return errors.Wrapf(err, "copy file %v to %v", msg.File, tsfile)
547-
}
548-
549-
// Get the file size.
550-
stats, err := os.Stat(msg.File)
551-
if err != nil {
552-
return errors.Wrapf(err, "stat file %v", msg.File)
544+
if file, err := os.Create(tsfile); err != nil {
545+
return errors.Wrapf(err, "create file %v error", tsfile)
546+
} else {
547+
defer file.Close()
548+
io.Copy(file, bytes.NewReader(data))
553549
}
554550

555551
// Create a local ts file object.
@@ -558,7 +554,7 @@ func (v *RecordWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage)
558554
URL: msg.URL,
559555
SeqNo: msg.SeqNo,
560556
Duration: msg.Duration,
561-
Size: uint64(stats.Size()),
557+
Size: uint64(len(data)),
562558
File: tsfile,
563559
}
564560

‎platform/dvr-tencent-cos.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44
package main
55

66
import (
7+
"bytes"
78
"context"
89
"encoding/json"
910
"fmt"
11+
"io"
1012
"net/http"
1113
"net/url"
1214
"os"
13-
"os/exec"
1415
"path"
1516
"strings"
1617
"sync"
@@ -231,7 +232,7 @@ func (v *DvrWorker) Handle(ctx context.Context, handler *http.ServeMux) error {
231232
return nil
232233
}
233234

234-
func (v *DvrWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) error {
235+
func (v *DvrWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage, data []byte) error {
235236
// Ignore for Tencent Cloud credentials not ready.
236237
if !v.ready() {
237238
return nil
@@ -241,16 +242,11 @@ func (v *DvrWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) er
241242
tsid := uuid.NewString()
242243
tsfile := path.Join("dvr", fmt.Sprintf("%v.ts", tsid))
243244

244-
// Always use execFile when params contains user inputs, see https://auth0.com/blog/preventing-command-injection-attacks-in-node-js-apps/
245-
// Note that should never use fs.copyFileSync(file, tsfile, fs.constants.COPYFILE_FICLONE_FORCE) which fails in macOS.
246-
if err := exec.CommandContext(ctx, "cp", "-f", msg.File, tsfile).Run(); err != nil {
247-
return errors.Wrapf(err, "copy file %v to %v", msg.File, tsfile)
248-
}
249-
250-
// Get the file size.
251-
stats, err := os.Stat(msg.File)
252-
if err != nil {
253-
return errors.Wrapf(err, "stat file %v", msg.File)
245+
if file, err := os.Create(tsfile); err != nil {
246+
return errors.Wrapf(err, "create file %v error", tsfile)
247+
} else {
248+
defer file.Close()
249+
io.Copy(file, bytes.NewReader(data))
254250
}
255251

256252
// Create a local ts file object.
@@ -259,7 +255,7 @@ func (v *DvrWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) er
259255
URL: msg.URL,
260256
SeqNo: msg.SeqNo,
261257
Duration: msg.Duration,
262-
Size: uint64(stats.Size()),
258+
Size: uint64(len(data)),
263259
File: tsfile,
264260
}
265261

‎platform/dvr-tencent-vod.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44
package main
55

66
import (
7+
"bytes"
78
"context"
89
"encoding/json"
910
"fmt"
11+
"io"
1012
"net/http"
1113
"net/url"
1214
"os"
13-
"os/exec"
1415
"path"
1516
"strconv"
1617
"strings"
@@ -321,7 +322,7 @@ func (v *VodWorker) Handle(ctx context.Context, handler *http.ServeMux) error {
321322
return nil
322323
}
323324

324-
func (v *VodWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) error {
325+
func (v *VodWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage, data []byte) error {
325326
// Ignore for Tencent Cloud credentials not ready.
326327
if !v.ready() {
327328
return nil
@@ -331,16 +332,11 @@ func (v *VodWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) er
331332
tsid := uuid.NewString()
332333
tsfile := path.Join("vod", fmt.Sprintf("%v.ts", tsid))
333334

334-
// Always use execFile when params contains user inputs, see https://auth0.com/blog/preventing-command-injection-attacks-in-node-js-apps/
335-
// Note that should never use fs.copyFileSync(file, tsfile, fs.constants.COPYFILE_FICLONE_FORCE) which fails in macOS.
336-
if err := exec.CommandContext(ctx, "cp", "-f", msg.File, tsfile).Run(); err != nil {
337-
return errors.Wrapf(err, "copy file %v to %v", msg.File, tsfile)
338-
}
339-
340-
// Get the file size.
341-
stats, err := os.Stat(msg.File)
342-
if err != nil {
343-
return errors.Wrapf(err, "stat file %v", msg.File)
335+
if file, err := os.Create(tsfile); err != nil {
336+
return errors.Wrapf(err, "create file %v error", tsfile)
337+
} else {
338+
defer file.Close()
339+
io.Copy(file, bytes.NewReader(data))
344340
}
345341

346342
// Create a local ts file object.
@@ -349,7 +345,7 @@ func (v *VodWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) er
349345
URL: msg.URL,
350346
SeqNo: msg.SeqNo,
351347
Duration: msg.Duration,
352-
Size: uint64(stats.Size()),
348+
Size: uint64(len(data)),
353349
File: tsfile,
354350
}
355351

‎platform/ocr.go

Lines changed: 9 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package main
55

66
import (
7+
"bytes"
78
"context"
89
"encoding/base64"
910
"encoding/json"
@@ -21,6 +22,7 @@ import (
2122
"github.com/ossrs/go-oryx-lib/errors"
2223
ohttp "github.com/ossrs/go-oryx-lib/http"
2324
"github.com/ossrs/go-oryx-lib/logger"
25+
2426
// Use v8 because we use Go 1.16+, while v9 requires Go 1.18+
2527
"github.com/go-redis/redis/v8"
2628
"github.com/google/uuid"
@@ -39,17 +41,12 @@ type OCRWorker struct {
3941
// The global OCR task, only support one OCR task.
4042
task *OCRTask
4143

42-
// Use async goroutine to process on_hls messages.
43-
msgs chan *SrsOnHlsMessage
44-
4544
// Got message from SRS, a new TS segment file is generated.
4645
tsfiles chan *SrsOnHlsObject
4746
}
4847

4948
func NewOCRWorker() *OCRWorker {
5049
v := &OCRWorker{
51-
// Message on_hls.
52-
msgs: make(chan *SrsOnHlsMessage, 1024),
5350
// TS files.
5451
tsfiles: make(chan *SrsOnHlsObject, 1024),
5552
}
@@ -547,16 +544,7 @@ func (v *OCRWorker) Enabled() bool {
547544
return v.task.enabled()
548545
}
549546

550-
func (v *OCRWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage) error {
551-
select {
552-
case <-ctx.Done():
553-
case v.msgs <- msg:
554-
}
555-
556-
return nil
557-
}
558-
559-
func (v *OCRWorker) OnHlsTsMessageImpl(ctx context.Context, msg *SrsOnHlsMessage) error {
547+
func (v *OCRWorker) OnHlsTsMessage(ctx context.Context, msg *SrsOnHlsMessage, data []byte) error {
560548
// Ignore if not natch the task config.
561549
if !v.task.match(msg) {
562550
return nil
@@ -566,16 +554,11 @@ func (v *OCRWorker) OnHlsTsMessageImpl(ctx context.Context, msg *SrsOnHlsMessage
566554
tsid := fmt.Sprintf("%v-org-%v", msg.SeqNo, uuid.NewString())
567555
tsfile := path.Join("ocr", fmt.Sprintf("%v.ts", tsid))
568556

569-
// Always use execFile when params contains user inputs, see https://auth0.com/blog/preventing-command-injection-attacks-in-node-js-apps/
570-
// Note that should never use fs.copyFileSync(file, tsfile, fs.constants.COPYFILE_FICLONE_FORCE) which fails in macOS.
571-
if err := exec.CommandContext(ctx, "cp", "-f", msg.File, tsfile).Run(); err != nil {
572-
return errors.Wrapf(err, "copy file %v to %v", msg.File, tsfile)
573-
}
574-
575-
// Get the file size.
576-
stats, err := os.Stat(msg.File)
577-
if err != nil {
578-
return errors.Wrapf(err, "stat file %v", msg.File)
557+
if file, err := os.Create(tsfile); err != nil {
558+
return errors.Wrapf(err, "create file %v error", tsfile)
559+
} else {
560+
defer file.Close()
561+
io.Copy(file, bytes.NewReader(data))
579562
}
580563

581564
// Create a local ts file object.
@@ -584,7 +567,7 @@ func (v *OCRWorker) OnHlsTsMessageImpl(ctx context.Context, msg *SrsOnHlsMessage
584567
URL: msg.URL,
585568
SeqNo: msg.SeqNo,
586569
Duration: msg.Duration,
587-
Size: uint64(stats.Size()),
570+
Size: uint64(len(data)),
588571
File: tsfile,
589572
}
590573

@@ -659,22 +642,6 @@ func (v *OCRWorker) Start(ctx context.Context) error {
659642
}
660643
}()
661644

662-
// Consume all on_hls messages.
663-
wg.Add(1)
664-
go func() {
665-
defer wg.Done()
666-
667-
for ctx.Err() == nil {
668-
select {
669-
case <-ctx.Done():
670-
case msg := <-v.msgs:
671-
if err := v.OnHlsTsMessageImpl(ctx, msg); err != nil {
672-
logger.Wf(ctx, "ocr: handle on hls message %v err %+v", msg.String(), err)
673-
}
674-
}
675-
}
676-
}()
677-
678645
// Consume all ts files by task.
679646
wg.Add(1)
680647
go func() {

‎platform/srs-hooks.go

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -730,46 +730,58 @@ func handleOnHls(ctx context.Context, handler *http.ServeMux) error {
730730
return errors.Errorf("invalid action=%v", msg.Action)
731731
}
732732

733-
if _, err := os.Stat(msg.File); err != nil {
734-
logger.Tf(ctx, "invalid ts file %v", msg.File)
733+
allowedDir, err := os.Getwd()
734+
if err != nil {
735+
return errors.Wrapf(err, "can not get current working directory")
736+
}
735737

736-
if err := os.MkdirAll(filepath.Dir(msg.File), 0755); err != nil {
737-
return errors.Wrapf(err, "failed to create ts file directory %v", filepath.Dir(msg.File))
738-
}
738+
safePath := filepath.Join(allowedDir, filepath.Clean(msg.File))
739+
logger.Tf(ctx, "safePath is %v", safePath)
740+
absPath, err := filepath.Abs(safePath)
741+
if err != nil {
742+
return errors.Wrapf(err, "can not get absolute path from %v", safePath)
743+
}
739744

740-
if tsFile, err := os.Create(msg.File); err != nil {
741-
return errors.Wrapf(err, "failed to create ts file %v", msg.File)
742-
} else {
743-
tsUrl := "http://" + os.Getenv("SRS_HOST") + ":" + os.Getenv("SRS_HTTP_STREAM_PORT") + "/" + msg.URL
744-
logger.Tf(ctx, "download ts from %v", tsUrl)
745-
client := http.Client{
746-
CheckRedirect: func(req *http.Request, via []*http.Request) error {
747-
r.URL.Opaque = r.URL.Path
748-
return nil
749-
},
750-
}
745+
if !filepath.HasPrefix(absPath, allowedDir) {
746+
return errors.Errorf("Access denied, %v is outside allowed directory", absPath)
747+
}
751748

752-
resp, err := client.Get(tsUrl)
753-
if err != nil {
754-
return errors.Wrapf(err, "http error to get url %v", tsUrl)
755-
}
756-
defer resp.Body.Close()
749+
var data []byte
750+
if _, err := os.Stat(safePath); err != nil {
751+
logger.Tf(ctx, "invalid ts file %v", safePath)
752+
tsUrl := "http://" + os.Getenv("SRS_HOST") + ":" + os.Getenv("SRS_HTTP_STREAM_PORT") + "/" + msg.URL
753+
logger.Tf(ctx, "download ts from %v", tsUrl)
754+
client := http.Client{
755+
CheckRedirect: func(req *http.Request, via []*http.Request) error {
756+
r.URL.Opaque = r.URL.Path
757+
return nil
758+
},
759+
}
757760

758-
size, err := io.Copy(tsFile, resp.Body)
759-
if err != nil {
760-
return errors.Wrapf(err, "copy http resp to file %v", tsFile)
761-
}
762-
defer tsFile.Close()
763-
logger.Tf(ctx, "Download ts file %s with size %d", tsUrl, size)
761+
res, err := client.Get(tsUrl)
762+
if err != nil {
763+
return errors.Wrapf(err, "http error to get url %v", tsUrl)
764+
}
765+
defer res.Body.Close()
766+
767+
if b, err := io.ReadAll(res.Body); err != nil {
768+
return errors.Wrapf(err, "read http response error")
769+
} else {
770+
data = b
764771
}
772+
logger.Tf(ctx, "Download ts file %s with size %d", tsUrl, len(data))
773+
} else if b, err := os.ReadFile(safePath); err != nil {
774+
return errors.Wrapf(err, "read %v error", safePath)
775+
} else {
776+
data = b
765777
}
766778
logger.Tf(ctx, "on_hls ok, %v", string(b))
767779

768780
// Handle TS file by Record task if enabled.
769781
if recordAll, err := rdb.HGet(ctx, SRS_RECORD_PATTERNS, "all").Result(); err != nil && err != redis.Nil {
770782
return errors.Wrapf(err, "hget %v all", SRS_RECORD_PATTERNS)
771783
} else if recordAll == "true" {
772-
if err = recordWorker.OnHlsTsMessage(ctx, &msg); err != nil {
784+
if err = recordWorker.OnHlsTsMessage(ctx, &msg, data); err != nil {
773785
return errors.Wrapf(err, "feed %v", msg.String())
774786
}
775787
logger.Tf(ctx, "record %v", msg.String())
@@ -779,7 +791,7 @@ func handleOnHls(ctx context.Context, handler *http.ServeMux) error {
779791
if dvrAll, err := rdb.HGet(ctx, SRS_DVR_PATTERNS, "all").Result(); err != nil && err != redis.Nil {
780792
return errors.Wrapf(err, "hget %v all", SRS_DVR_PATTERNS)
781793
} else if dvrAll == "true" {
782-
if err = dvrWorker.OnHlsTsMessage(ctx, &msg); err != nil {
794+
if err = dvrWorker.OnHlsTsMessage(ctx, &msg, data); err != nil {
783795
return errors.Wrapf(err, "feed %v", msg.String())
784796
}
785797
logger.Tf(ctx, "dvr %v", msg.String())
@@ -789,23 +801,23 @@ func handleOnHls(ctx context.Context, handler *http.ServeMux) error {
789801
if vodAll, err := rdb.HGet(ctx, SRS_VOD_PATTERNS, "all").Result(); err != nil && err != redis.Nil {
790802
return errors.Wrapf(err, "hget %v all", SRS_VOD_PATTERNS)
791803
} else if vodAll == "true" {
792-
if err = vodWorker.OnHlsTsMessage(ctx, &msg); err != nil {
804+
if err = vodWorker.OnHlsTsMessage(ctx, &msg, data); err != nil {
793805
return errors.Wrapf(err, "feed %v", msg.String())
794806
}
795807
logger.Tf(ctx, "vod %v", msg.String())
796808
}
797809

798810
// Handle TS file by Transcript task if enabled.
799811
if transcriptWorker.Enabled() {
800-
if err = transcriptWorker.OnHlsTsMessage(ctx, &msg); err != nil {
812+
if err = transcriptWorker.OnHlsTsMessage(ctx, &msg, data); err != nil {
801813
return errors.Wrapf(err, "feed %v", msg.String())
802814
}
803815
logger.Tf(ctx, "transcript %v", msg.String())
804816
}
805817

806818
// Handle TS file by OCR task if enabled.
807819
if ocrWorker.Enabled() {
808-
if err = ocrWorker.OnHlsTsMessage(ctx, &msg); err != nil {
820+
if err = ocrWorker.OnHlsTsMessage(ctx, &msg, data); err != nil {
809821
return errors.Wrapf(err, "feed %v", msg.String())
810822
}
811823
logger.Tf(ctx, "ocr %v", msg.String())

0 commit comments

Comments
 (0)