[security] fix vulnerability when handling remote file redirects

Also adds the ability for an admin to just completely disable this service if it is not needed on the node.

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v1.2.3
+### Fixed
+* **[Security]** Fixes a remaining security vulnerability in the code handling remote file downloads for servers relating to redirect validation.
+
+### Added
+* Adds a configuration key at `api.disable_remote_download` that can be set to `true` to completely download the remote download system.
+
 ## v1.2.2
 ### Fixed
 * Reverts changes to logic handling blocking until a server process is done running when polling stats. This change exposed a bug in the underlying Docker system causing servers to enter a state in which Wings was unable to terminate the process and Docker commands would hang if executed against the container.

config/config.go

@@ -88,11 +88,16 @@ type ApiConfiguration struct {
 
  // SSL configuration for the daemon.
  Ssl struct {
- Enabled bool `default:"false"`
+ Enabled bool `json:"enabled" yaml:"enabled"`
  CertificateFile string `json:"cert" yaml:"cert"`
  KeyFile string `json:"key" yaml:"key"`
  }
 
+ // Determines if functionality for allowing remote download of files into server directories
+ // is enabled on this instance. If set to "true" remote downloads will not be possible for
+ // servers.
+ DisableRemoteDownload bool `json:"disable_remote_download" yaml:"disable_remote_download"`
+
  // The maximum size for files uploaded through the Panel in bytes.
  UploadLimit int `default:"100" json:"upload_limit" yaml:"upload_limit"`
 }

router/downloader/downloader.go

@@ -18,7 +18,22 @@ import (
  "time"
 )
 
-var client = &http.Client{Timeout: time.Hour * 12}
+var client = &http.Client{
+ Timeout: time.Hour * 12,
+ // Disallow any redirect on a HTTP call. This is a security requirement: do not modify
+ // this logic without first ensuring that the new target location IS NOT within the current
+ // instance's local network.
+ //
+ // This specific error response just causes the client to not follow the redirect and
+ // returns the actual redirect response to the caller. Not perfect, but simple and most
+ // people won't be using URLs that redirect anyways hopefully?
+ //
+ // We'll re-evaluate this down the road if needed.
+ CheckRedirect: func(req *http.Request, via []*http.Request) error {
+ return http.ErrUseLastResponse
+ },
+}
+
 var instance = &Downloader{
  // Tracks all of the active downloads.
  downloadCache: make(map[string]*Download),

router/middleware.go

@@ -120,6 +120,21 @@ func (m *Middleware) ServerExists() gin.HandlerFunc {
  }
 }
 
+// Checks if remote file downloading is enabled on this instance before allowing access
+// to the given endpoint.
+func (m *Middleware) CheckRemoteDownloadEnabled() gin.HandlerFunc {
+ disabled := config.Get().Api.DisableRemoteDownload
+ return func(c *gin.Context) {
+ if disabled {
+ c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{
+ "error": "This functionality is not currently enabled on this instance.",
+ })
+ return
+ }
+ c.Next()
+ }
+}
+
 // Returns the server instance from the gin context. If there is no server set in the
 // context (e.g. calling from a controller not protected by ServerExists) this function
 // will panic.

router/router.go

@@ -88,9 +88,9 @@ func Configure() *gin.Engine {
  files.POST("/decompress", postServerDecompressFiles)
  files.POST("/chmod", postServerChmodFile)
 
- files.GET("/pull", getServerPullingFiles)
- files.POST("/pull", postServerPullRemoteFile)
- files.DELETE("/pull/:download", deleteServerPullRemoteFile)
+ files.GET("/pull", m.CheckRemoteDownloadEnabled(), getServerPullingFiles)
+ files.POST("/pull", m.CheckRemoteDownloadEnabled(), postServerPullRemoteFile)
+ files.DELETE("/pull/:download", m.CheckRemoteDownloadEnabled(), deleteServerPullRemoteFile)
  }
 
  backup := server.Group("/backup")