diff --git a/config/config.go b/config/config.go index 3fbb6148..d2ec71ed 100644 --- a/config/config.go +++ b/config/config.go @@ -79,6 +79,8 @@ var ( AllowedSources []*regexp.Regexp + SanitizeSvg bool + CookiePassthrough bool CookieBaseURL string @@ -237,6 +239,8 @@ func Reset() { AllowedSources = make([]*regexp.Regexp, 0) + SanitizeSvg = true + CookiePassthrough = false CookieBaseURL = "" @@ -344,6 +348,8 @@ func Configure() error { configurators.Patterns(&AllowedSources, "IMGPROXY_ALLOWED_SOURCES") + configurators.Bool(&SanitizeSvg, "IMGPROXY_SANITIZE_SVG") + configurators.Bool(&JpegProgressive, "IMGPROXY_JPEG_PROGRESSIVE") configurators.Bool(&PngInterlaced, "IMGPROXY_PNG_INTERLACED") configurators.Bool(&PngQuantize, "IMGPROXY_PNG_QUANTIZE") diff --git a/docs/configuration.md b/docs/configuration.md index f6ea32b4..3d7ecf3e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -91,6 +91,8 @@ You can limit allowed source URLs with the following variable: * Good: `http://example.com/` If the trailing slash is absent, `http://example.com@baddomain.com` would be a permissable URL, however, the request would be made to `baddomain.com`. +* `IMGPROXY_SANITIZE_SVG`: when true, imgproxy will remove scripts from SVG images to prevent XSS attacks. Defaut: `true` + When using imgproxy in a development environment, it can be useful to ignore SSL verification: * `IMGPROXY_IGNORE_SSL_VERIFICATION`: when true, disables SSL verification, so imgproxy can be used in a development environment with self-signed SSL certificates. diff --git a/go.mod b/go.mod index ce68ac6c..ace9332c 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/prometheus/client_golang v1.12.1 github.com/sirupsen/logrus v1.8.1 github.com/stretchr/testify v1.7.1 + github.com/tdewolff/parse/v2 v2.6.0 // indirect github.com/trimmer-io/go-xmp v1.0.0 go.uber.org/automaxprocs v1.5.1 golang.org/x/image v0.0.0-20220413100746-70e8d0d3baa9 diff --git a/go.sum b/go.sum index 9e0c7069..2d0c846d 100644 --- a/go.sum +++ b/go.sum @@ -1078,6 +1078,9 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/syndtr/goleveldb v0.0.0-20160425020131-cfa635847112/go.mod h1:Z4AUp2Km+PwemOoO/VB5AOx9XSsIItzFjoJlOSiYmn0= github.com/syndtr/goleveldb v1.0.0/go.mod h1:ZVVdQEZoIme9iO1Ch2Jdy24qqXrMMOU6lpPAyBWyWuQ= +github.com/tdewolff/parse/v2 v2.6.0 h1:f2D7w32JtqjCv6SczWkfwK+m15et42qEtDnZXHoNY70= +github.com/tdewolff/parse/v2 v2.6.0/go.mod h1:WzaJpRSbwq++EIQHYIRTpbYKNA3gn9it1Ik++q4zyho= +github.com/tdewolff/test v1.0.6/go.mod h1:6DAvZliBAAnD7rhVgwaM7DE5/d9NMOAJ09SqYqeK4QE= github.com/tidwall/btree v0.3.0/go.mod h1:huei1BkDWJ3/sLXmO+bsCNELL+Bp2Kks9OLyQFkzvA8= github.com/tidwall/btree v1.1.0/go.mod h1:TzIRzen6yHbibdSfK6t8QimqbUnoxUSrZfeW7Uob0q4= github.com/tidwall/buntdb v1.2.0/go.mod h1:XLza/dhlwzO6dc5o/KWor4kfZSt3BP8QV+77ZMKfI58= diff --git a/processing_handler.go b/processing_handler.go index 258dbcf1..450e598c 100644 --- a/processing_handler.go +++ b/processing_handler.go @@ -22,6 +22,7 @@ import ( "github.com/imgproxy/imgproxy/v3/processing" "github.com/imgproxy/imgproxy/v3/router" "github.com/imgproxy/imgproxy/v3/security" + "github.com/imgproxy/imgproxy/v3/svg" "github.com/imgproxy/imgproxy/v3/vips" ) @@ -275,6 +276,22 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) { if originData.Type == po.Format || po.Format == imagetype.Unknown { // Don't process SVG if originData.Type == imagetype.SVG { + if config.SanitizeSvg { + sanitized, svgErr := svg.Satitize(originData.Data) + if svgErr != nil { + panic(svgErr) + } + + // Since we'll replace origin data, it's better to close it to return + // it's buffer to the pool + originData.Close() + + originData = &imagedata.ImageData{ + Data: sanitized, + Type: imagetype.SVG, + } + } + respondWithImage(reqID, r, rw, statusCode, originData, po, imageURL, originData) return } diff --git a/processing_handler_test.go b/processing_handler_test.go index af062739..42609c8d 100644 --- a/processing_handler_test.go +++ b/processing_handler_test.go @@ -20,6 +20,7 @@ import ( "github.com/imgproxy/imgproxy/v3/imagetype" "github.com/imgproxy/imgproxy/v3/options" "github.com/imgproxy/imgproxy/v3/router" + "github.com/imgproxy/imgproxy/v3/svg" "github.com/imgproxy/imgproxy/v3/vips" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -289,7 +290,9 @@ func (s *ProcessingHandlerTestSuite) TestSkipProcessingSVG() { assert.Equal(s.T(), 200, res.StatusCode) actual := s.readBody(res) - expected := s.readTestFile("test1.svg") + expected, err := svg.Satitize(s.readTestFile("test1.svg")) + + assert.Nil(s.T(), err) assert.True(s.T(), bytes.Equal(expected, actual)) } diff --git a/svg/svg.go b/svg/svg.go new file mode 100644 index 00000000..d3178a06 --- /dev/null +++ b/svg/svg.go @@ -0,0 +1,56 @@ +package svg + +import ( + "bytes" + "io" + "strings" + + "github.com/tdewolff/parse/v2" + "github.com/tdewolff/parse/v2/xml" +) + +func Satitize(data []byte) ([]byte, error) { + r := bytes.NewReader(data) + l := xml.NewLexer(parse.NewInput(r)) + + buf := new(bytes.Buffer) + buf.Grow(len(data)) + + ignoreTag := 0 + + for { + tt, tdata := l.Next() + + if ignoreTag > 0 { + switch tt { + case xml.EndTagToken, xml.StartTagCloseVoidToken: + ignoreTag-- + case xml.StartTagToken: + ignoreTag++ + } + + continue + } + + switch tt { + case xml.ErrorToken: + if l.Err() != io.EOF { + return nil, l.Err() + } + return buf.Bytes(), nil + case xml.StartTagToken: + if strings.ToLower(string(l.Text())) == "script" { + ignoreTag++ + continue + } + buf.Write(tdata) + case xml.AttributeToken: + if strings.ToLower(string(l.Text())) == "onload" { + continue + } + buf.Write(tdata) + default: + buf.Write(tdata) + } + } +}