[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.
This commit is contained in:
Dane Everitt 2021-01-09 17:52:27 -08:00
parent 6701aa6dc1
commit 96256ac63e
No known key found for this signature in database
GPG Key ID: EEA66103B3D71F53
5 changed files with 47 additions and 5 deletions

View File

@ -1,5 +1,12 @@
# Changelog # 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 ## v1.2.2
### Fixed ### 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. * 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.

View File

@ -88,11 +88,16 @@ type ApiConfiguration struct {
// SSL configuration for the daemon. // SSL configuration for the daemon.
Ssl struct { Ssl struct {
Enabled bool `default:"false"` Enabled bool `json:"enabled" yaml:"enabled"`
CertificateFile string `json:"cert" yaml:"cert"` CertificateFile string `json:"cert" yaml:"cert"`
KeyFile string `json:"key" yaml:"key"` 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. // The maximum size for files uploaded through the Panel in bytes.
UploadLimit int `default:"100" json:"upload_limit" yaml:"upload_limit"` UploadLimit int `default:"100" json:"upload_limit" yaml:"upload_limit"`
} }

View File

@ -18,7 +18,22 @@ import (
"time" "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{ var instance = &Downloader{
// Tracks all of the active downloads. // Tracks all of the active downloads.
downloadCache: make(map[string]*Download), downloadCache: make(map[string]*Download),

View File

@ -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 // 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 // context (e.g. calling from a controller not protected by ServerExists) this function
// will panic. // will panic.

View File

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