prevent code injection in case of MTX_QUERY in hooks (#5707)
When MTX_QUERY is used explicitly in hooks, for instance "curl http://something/?$MTX_QUERY", it can be used to inject arbitrary commands. MTX_QUERY is now url-encoded to prevent any abuse regardless of the configuration.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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])
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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:]...)
|
||||
}
|
||||
|
||||
|
||||
@@ -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 != "" {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
+3
-3
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user