diff --git a/docs/2-features/21-hooks.md b/docs/2-features/21-hooks.md index f06e4a35..adb21c25 100644 --- a/docs/2-features/21-hooks.md +++ b/docs/2-features/21-hooks.md @@ -58,7 +58,7 @@ pathDefaults: # This is terminated with SIGINT when there are no readers anymore. # The following environment variables are available: # * MTX_PATH: path name - # * MTX_QUERY: query parameters (passed by first reader) + # * MTX_QUERY: query parameters (passed by first reader) (url-encoded) # * RTSP_PORT: RTSP server port # * G1, G2, ...: regular expression groups, if path name is # a regular expression. @@ -89,7 +89,7 @@ pathDefaults: # This is terminated with SIGINT when the stream is not ready anymore. # The following environment variables are available: # * MTX_PATH: path name - # * MTX_QUERY: query parameters (passed by publisher) + # * MTX_QUERY: query parameters (passed by publisher) (url-encoded) # * MTX_SOURCE_TYPE: source type # * MTX_SOURCE_ID: source ID # * RTSP_PORT: RTSP server port @@ -121,7 +121,7 @@ pathDefaults: # This is terminated with SIGINT when a client stops reading. # The following environment variables are available: # * MTX_PATH: path name - # * MTX_QUERY: query parameters (passed by reader) + # * MTX_QUERY: query parameters (passed by reader) (url-encoded) # * MTX_READER_TYPE: reader type # * MTX_READER_ID: reader ID # * RTSP_PORT: RTSP server port diff --git a/internal/core/path_test.go b/internal/core/path_test.go index 1ddb4fd9..c34db1b7 100644 --- a/internal/core/path_test.go +++ b/internal/core/path_test.go @@ -77,7 +77,7 @@ func TestPathRunOnDemand(t *testing.T) { if ca == "describe" || ca == "describe and setup" { var u *base.URL - u, err = base.ParseURL("rtsp://localhost:8554/ondemand?param=value") + u, err = base.ParseURL("rtsp://localhost:8554/ondemand?key1=val1&key2=val2") require.NoError(t, err) byts, _ := base.Request{ @@ -99,9 +99,9 @@ func TestPathRunOnDemand(t *testing.T) { err = desc.Unmarshal(res.Body) require.NoError(t, err) control, _ = desc.MediaDescriptions[0].Attribute("control") - control = "rtsp://localhost:8554/ondemand?param=value/" + control + control = "rtsp://localhost:8554/ondemand?key1=val1&key2=val2/" + control } else { - control = "rtsp://localhost:8554/ondemand?param=value/" + control = "rtsp://localhost:8554/ondemand?key1=val1&key2=val2/" } if ca == "setup" || ca == "describe and setup" { @@ -298,7 +298,7 @@ func TestPathRunOnReady(t *testing.T) { require.NoError(t, err) fields := strings.Split(string(byts[:len(byts)-1]), " ") require.Equal(t, "test", fields[0]) - require.Equal(t, "query=value", fields[1]) + require.Equal(t, "query%3Dvalue", fields[1]) require.Equal(t, "rtspSession", fields[2]) require.NotEmpty(t, fields[3]) require.Equal(t, "8554", fields[4]) @@ -308,13 +308,64 @@ func TestPathRunOnReady(t *testing.T) { require.NoError(t, err) fields = strings.Split(string(byts[:len(byts)-1]), " ") require.Equal(t, "test", fields[0]) - require.Equal(t, "query=value", fields[1]) + require.Equal(t, "query%3Dvalue", fields[1]) require.Equal(t, "rtspSession", fields[2]) require.NotEmpty(t, fields[3]) require.Equal(t, "8554", fields[4]) require.Equal(t, "st", fields[5]) } +func TestPathRunOnReadyQueryInjection(t *testing.T) { + sentinel := filepath.Join(t.TempDir(), "mediamtx_test_query_injection_sentinel") + + for _, ca := range []struct { + name string + cmdstr string + query string + }{ + { + // $(…) inside double quotes still triggers command substitution. + // ${IFS} expands to a space, avoiding a literal space in the query string. + name: "command substitution dollar", + cmdstr: "sh -c 'echo \"$MTX_QUERY\"'", + query: "$(touch${IFS}" + sentinel + "1)", + }, + { + // > in an unquoted expansion redirects echo's output, creating the file. + name: "redirect", + cmdstr: "sh -c 'echo $MTX_QUERY'", + query: ">" + sentinel + "2", + }, + { + // & in an unquoted expansion backgrounds echo and runs touch directly. + // ${IFS} expands to a space, avoiding a literal space in the query string. + name: "and operator", + cmdstr: "sh -c 'echo $MTX_QUERY'", + query: "&touch${IFS}" + sentinel + "3", + }, + } { + t.Run(ca.name, func(t *testing.T) { + p, ok := newInstance(t, fmt.Sprintf( + "rtmp: no\nhls: no\nwebrtc: no\npaths:\n test:\n runOnReady: %s\n", + ca.cmdstr)) + require.Equal(t, true, ok) + defer p.Close() + + c := gortsplib.Client{} + err := c.StartRecording( + "rtsp://localhost:8554/test?"+ca.query, + &description.Session{Medias: []*description.Media{test.UniqueMediaH264()}}) + require.NoError(t, err) + defer c.Close() + + time.Sleep(500 * time.Millisecond) + + _, statErr := os.Stat(sentinel) + require.ErrorIs(t, statErr, os.ErrNotExist) + }) + } +} + func TestPathRunOnRead(t *testing.T) { serverCertFpath := test.CreateTempFile(t, test.TLSCertPub) serverKeyFpath := test.CreateTempFile(t, test.TLSCertKey) @@ -546,7 +597,7 @@ func TestPathRunOnRead(t *testing.T) { require.NoError(t, err) fields := strings.Split(string(byts[:len(byts)-1]), " ") require.Equal(t, "test", fields[0]) - require.Equal(t, "query=value", fields[1]) + require.Equal(t, "query%3Dvalue", fields[1]) require.Equal(t, readerType, fields[2]) require.NotEmpty(t, fields[3]) require.Equal(t, "8554", fields[4]) @@ -556,7 +607,7 @@ func TestPathRunOnRead(t *testing.T) { require.NoError(t, err) fields = strings.Split(string(byts[:len(byts)-1]), " ") require.Equal(t, "test", fields[0]) - require.Equal(t, "query=value", fields[1]) + require.Equal(t, "query%3Dvalue", fields[1]) require.Equal(t, readerType, fields[2]) require.NotEmpty(t, fields[3]) require.Equal(t, "8554", fields[4]) diff --git a/internal/core/test_on_demand/main.go b/internal/core/test_on_demand/main.go index baba11dd..1789dd70 100644 --- a/internal/core/test_on_demand/main.go +++ b/internal/core/test_on_demand/main.go @@ -12,11 +12,11 @@ import ( ) func main() { - if os.Getenv("MTX_QUERY") != "param=value" { - panic("unexpected MTX_QUERY") + if os.Getenv("MTX_QUERY") != "key1%3Dval1%26key2%3Dval2" { + panic("unexpected MTX_QUERY: " + os.Getenv("MTX_QUERY")) } if os.Getenv("G1") != "on" { - panic("unexpected G1") + panic("unexpected G1: " + os.Getenv("G1")) } medi := &description.Media{ diff --git a/internal/externalcmd/cmd.go b/internal/externalcmd/cmd.go index 18cfb72a..5492808a 100644 --- a/internal/externalcmd/cmd.go +++ b/internal/externalcmd/cmd.go @@ -50,17 +50,17 @@ func (c *Cmd) Close() { close(c.terminate) } -func (c *Cmd) run() { - defer c.Pool.wg.Done() - - // replace variables in both Linux and Windows, in order to allow using the - // same commands on both of them. - cmdstr := os.Expand(c.Cmdstr, func(variable string) string { - if value, ok := c.Env[variable]; ok { +func expandEnv(s string, env Environment) string { + return os.Expand(s, func(variable string) string { + if value, ok := env[variable]; ok { return value } return os.Getenv(variable) }) +} + +func (c *Cmd) run() { + defer c.Pool.wg.Done() env := append([]string(nil), os.Environ()...) for key, val := range c.Env { @@ -68,7 +68,7 @@ func (c *Cmd) run() { } for { - err := c.runOSSpecific(cmdstr, env) + err := c.runOSSpecific(c.Cmdstr, env) if errors.Is(err, errTerminated) { return } diff --git a/internal/externalcmd/cmd_test.go b/internal/externalcmd/cmd_test.go new file mode 100644 index 00000000..ee6ac219 --- /dev/null +++ b/internal/externalcmd/cmd_test.go @@ -0,0 +1,46 @@ +//go:build !windows + +package externalcmd + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestCmdRunExpandAfterSplit(t *testing.T) { + // if os.Expand runs before shellquote.Split, a variable value containing a + // single quote produces unbalanced quotes that cause shellquote.Split to fail. + p := &Pool{} + p.Initialize() + + out := filepath.Join(t.TempDir(), "out") + + cmd := &Cmd{ + Pool: p, + Cmdstr: "sh -c 'echo \"$MY_VAR\" > " + out + "'", + Env: Environment{ + "MY_VAR": "it's", + }, + } + cmd.Start() + + poolClosed := make(chan struct{}) + go func() { + p.Close() + close(poolClosed) + }() + + select { + case <-poolClosed: + case <-time.After(10 * time.Second): + t.Fatal("timeout") + } + + byts, err := os.ReadFile(out) + require.NoError(t, err) + require.Equal(t, "it's\n", string(byts)) +} diff --git a/internal/externalcmd/cmd_unix.go b/internal/externalcmd/cmd_unix.go index 59003d5a..06ecda74 100644 --- a/internal/externalcmd/cmd_unix.go +++ b/internal/externalcmd/cmd_unix.go @@ -18,6 +18,10 @@ func (c *Cmd) runOSSpecific(cmdstr string, env []string) error { return err } + for i, part := range cmdParts { + cmdParts[i] = expandEnv(part, c.Env) + } + cmd := exec.Command(cmdParts[0], cmdParts[1:]...) cmd.Env = env diff --git a/internal/externalcmd/cmd_win.go b/internal/externalcmd/cmd_win.go index 1fc0e07b..98bc7e33 100644 --- a/internal/externalcmd/cmd_win.go +++ b/internal/externalcmd/cmd_win.go @@ -73,6 +73,7 @@ func (c *Cmd) runOSSpecific(cmdstr string, env []string) error { // line in SysProcAttr.CmdLine, leaving Args empty. if strings.HasPrefix(cmdstr, "cmd ") || strings.HasPrefix(cmdstr, "cmd.exe ") { args := strings.TrimPrefix(strings.TrimPrefix(cmdstr, "cmd "), "cmd.exe ") + args = expandEnv(args, c.Env) cmd = exec.Command("cmd.exe") cmd.SysProcAttr = &syscall.SysProcAttr{ @@ -84,6 +85,10 @@ func (c *Cmd) runOSSpecific(cmdstr string, env []string) error { return err } + for i, part := range cmdParts { + cmdParts[i] = expandEnv(part, c.Env) + } + cmd = exec.Command(cmdParts[0], cmdParts[1:]...) } diff --git a/internal/hooks/on_demand.go b/internal/hooks/on_demand.go index fb413a1a..d1433951 100644 --- a/internal/hooks/on_demand.go +++ b/internal/hooks/on_demand.go @@ -1,6 +1,8 @@ package hooks import ( + "net/url" + "github.com/bluenviron/mediamtx/internal/conf" "github.com/bluenviron/mediamtx/internal/externalcmd" "github.com/bluenviron/mediamtx/internal/logger" @@ -22,7 +24,7 @@ func OnDemand(params OnDemandParams) func(string) { if params.Conf.RunOnDemand != "" || params.Conf.RunOnUnDemand != "" { env = params.ExternalCmdEnv - env["MTX_QUERY"] = params.Query + env["MTX_QUERY"] = url.QueryEscape(params.Query) } if params.Conf.RunOnDemand != "" { diff --git a/internal/hooks/on_read.go b/internal/hooks/on_read.go index 8ab82d6b..39b5e9c0 100644 --- a/internal/hooks/on_read.go +++ b/internal/hooks/on_read.go @@ -1,6 +1,8 @@ package hooks import ( + "net/url" + "github.com/bluenviron/mediamtx/internal/conf" "github.com/bluenviron/mediamtx/internal/defs" "github.com/bluenviron/mediamtx/internal/externalcmd" @@ -25,7 +27,7 @@ func OnRead(params OnReadParams) func() { if params.Conf.RunOnRead != "" || params.Conf.RunOnUnread != "" { env = params.ExternalCmdEnv desc := params.Reader - env["MTX_QUERY"] = params.Query + env["MTX_QUERY"] = url.QueryEscape(params.Query) env["MTX_READER_TYPE"] = string(desc.Type) env["MTX_READER_ID"] = desc.ID } diff --git a/internal/hooks/on_ready.go b/internal/hooks/on_ready.go index 16a06b09..0cf24792 100644 --- a/internal/hooks/on_ready.go +++ b/internal/hooks/on_ready.go @@ -1,6 +1,8 @@ package hooks import ( + "net/url" + "github.com/bluenviron/mediamtx/internal/conf" "github.com/bluenviron/mediamtx/internal/defs" "github.com/bluenviron/mediamtx/internal/externalcmd" @@ -24,7 +26,7 @@ func OnReady(params OnReadyParams) func() { if params.Conf.RunOnReady != "" || params.Conf.RunOnNotReady != "" { env = params.ExternalCmdEnv - env["MTX_QUERY"] = params.Query + env["MTX_QUERY"] = url.QueryEscape(params.Query) if params.Desc != nil { env["MTX_SOURCE_TYPE"] = string(params.Desc.Type) env["MTX_SOURCE_ID"] = params.Desc.ID diff --git a/mediamtx.yml b/mediamtx.yml index d12509a3..3c8610bc 100644 --- a/mediamtx.yml +++ b/mediamtx.yml @@ -712,7 +712,7 @@ pathDefaults: # This is terminated with SIGINT when there are no readers anymore. # The following environment variables are available: # * MTX_PATH: path name - # * MTX_QUERY: query parameters (passed by first reader) + # * MTX_QUERY: query parameters (passed by first reader) (url-encoded) # * RTSP_PORT: RTSP server port # * G1, G2, ...: regular expression groups, if path name is # a regular expression. @@ -734,7 +734,7 @@ pathDefaults: # This is terminated with SIGINT when the stream is not ready anymore. # The following environment variables are available: # * MTX_PATH: path name - # * MTX_QUERY: query parameters (passed by publisher) + # * MTX_QUERY: query parameters (passed by publisher) (url-encoded) # * MTX_SOURCE_TYPE: source type # * MTX_SOURCE_ID: source ID # * RTSP_PORT: RTSP server port @@ -751,7 +751,7 @@ pathDefaults: # This is terminated with SIGINT when a client stops reading. # The following environment variables are available: # * MTX_PATH: path name - # * MTX_QUERY: query parameters (passed by reader) + # * MTX_QUERY: query parameters (passed by reader) (url-encoded) # * MTX_READER_TYPE: reader type # * MTX_READER_ID: reader ID # * RTSP_PORT: RTSP server port