fix: fix security vulnerabilities reported by CodeQL (#5467)

This commit is contained in:
Dean Sheather 2022-12-20 05:25:59 +10:00 committed by GitHub
parent e359f3cd23
commit 1bc4eb5329
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 42 additions and 22 deletions

View File

@ -32,7 +32,7 @@ func (lp *listeningPortsHandler) getListeningPorts() ([]codersdk.ListeningPort,
seen := make(map[uint16]struct{}, len(tabs))
ports := []codersdk.ListeningPort{}
for _, tab := range tabs {
if tab.LocalAddr == nil || tab.LocalAddr.Port < uint16(codersdk.MinimumListeningPort) {
if tab.LocalAddr == nil || tab.LocalAddr.Port < codersdk.MinimumListeningPort {
continue
}

View File

@ -8,6 +8,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"
"github.com/spf13/cobra"
@ -55,7 +56,7 @@ func CreateTemplateVersionSource(t *testing.T, responses *echo.Responses) string
directory := t.TempDir()
f, err := ioutil.TempFile(directory, "*.tf")
require.NoError(t, err)
f.Close()
_ = f.Close()
data, err := echo.Tar(responses)
require.NoError(t, err)
extractTar(t, data, directory)
@ -70,6 +71,9 @@ func extractTar(t *testing.T, data []byte, directory string) {
break
}
require.NoError(t, err)
if header.Name == "." || strings.Contains(header.Name, "..") {
continue
}
// #nosec
path := filepath.Join(directory, header.Name)
mode := header.FileInfo().Mode()

View File

@ -691,16 +691,17 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
}
client := codersdk.New(localURL)
if cfg.TLS.Enable.Value {
// Secure transport isn't needed for locally communicating!
if localURL.Scheme == "https" && isLocalhost(localURL.Hostname()) {
// The certificate will likely be self-signed or for a different
// hostname, so we need to skip verification.
client.HTTPClient.Transport = &http.Transport{
TLSClientConfig: &tls.Config{
//nolint:gosec
InsecureSkipVerify: true,
},
}
defer client.HTTPClient.CloseIdleConnections()
}
defer client.HTTPClient.CloseIdleConnections()
// This is helpful for tests, but can be silently ignored.
// Coder may be ran as users that don't have permission to write in the homedir,
@ -1404,7 +1405,7 @@ func startBuiltinPostgres(ctx context.Context, cfg config.Root, logger slog.Logg
if err != nil {
return "", nil, xerrors.Errorf("read postgres port: %w", err)
}
pgPort, err := strconv.Atoi(pgPortRaw)
pgPort, err := strconv.ParseUint(pgPortRaw, 10, 16)
if err != nil {
return "", nil, xerrors.Errorf("parse postgres port: %w", err)
}
@ -1465,3 +1466,9 @@ func redirectHTTPToAccessURL(handler http.Handler, accessURL *url.URL) http.Hand
handler.ServeHTTP(w, r)
})
}
// isLocalhost returns true if the host points to the local machine. Intended to
// be called with `u.Hostname()`.
func isLocalhost(host string) bool {
return host == "localhost" || host == "127.0.0.1" || host == "::1"
}

View File

@ -45,13 +45,13 @@ var scope = map[codersdk.GitProvider][]string{
codersdk.GitProviderGitHub: {"repo", "workflow"},
}
// regex provides defaults for each Git provider to
// match their SaaS host URL. This is configurable by each provider.
// regex provides defaults for each Git provider to match their SaaS host URL.
// This is configurable by each provider.
var regex = map[codersdk.GitProvider]*regexp.Regexp{
codersdk.GitProviderAzureDevops: regexp.MustCompile(`dev\.azure\.com`),
codersdk.GitProviderBitBucket: regexp.MustCompile(`bitbucket\.org`),
codersdk.GitProviderGitLab: regexp.MustCompile(`gitlab\.com`),
codersdk.GitProviderGitHub: regexp.MustCompile(`github\.com`),
codersdk.GitProviderAzureDevops: regexp.MustCompile(`^(https?://)?dev\.azure\.com(/.*)?$`),
codersdk.GitProviderBitBucket: regexp.MustCompile(`^(https?://)?bitbucket\.org(/.*)?$`),
codersdk.GitProviderGitLab: regexp.MustCompile(`^(https?://)?gitlab\.com(/.*)?$`),
codersdk.GitProviderGitHub: regexp.MustCompile(`^(https?://)?github\.com(/.*)?$`),
}
// newJWTOAuthConfig creates a new OAuth2 config that uses a custom

View File

@ -222,11 +222,11 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
})
return
}
height, err := strconv.Atoi(r.URL.Query().Get("height"))
height, err := strconv.ParseUint(r.URL.Query().Get("height"), 10, 16)
if err != nil {
height = 80
}
width, err := strconv.Atoi(r.URL.Query().Get("width"))
width, err := strconv.ParseUint(r.URL.Query().Get("width"), 10, 16)
if err != nil {
width = 80
}
@ -330,7 +330,7 @@ func (api *API) workspaceAgentListeningPorts(rw http.ResponseWriter, r *http.Req
if port == "" {
continue
}
portNum, err := strconv.Atoi(port)
portNum, err := strconv.ParseUint(port, 10, 16)
if err != nil {
continue
}
@ -344,7 +344,7 @@ func (api *API) workspaceAgentListeningPorts(rw http.ResponseWriter, r *http.Req
// common non-HTTP ports such as databases, FTP, SSH, etc.
filteredPorts := make([]codersdk.ListeningPort, 0, len(portsResponse.Ports))
for _, port := range portsResponse.Ports {
if port.Port < uint16(codersdk.MinimumListeningPort) {
if port.Port < codersdk.MinimumListeningPort {
continue
}
if _, ok := appPorts[port.Port]; ok {

View File

@ -27,7 +27,10 @@ var (
// TailnetIP is a static IPv6 address with the Tailscale prefix that is used to route
// connections from clients to this node. A dynamic address is not required because a Tailnet
// client only dials a single agent at a time.
TailnetIP = netip.MustParseAddr("fd7a:115c:a1e0:49d6:b259:b7ac:b1b2:48f4")
TailnetIP = netip.MustParseAddr("fd7a:115c:a1e0:49d6:b259:b7ac:b1b2:48f4")
)
const (
TailnetSSHPort = 1
TailnetReconnectingPTYPort = 2
TailnetSpeedtestPort = 3
@ -171,7 +174,7 @@ func (c *AgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, w
ctx, span := tracing.StartSpan(ctx)
defer span.End()
conn, err := c.DialContextTCP(ctx, netip.AddrPortFrom(TailnetIP, uint16(TailnetReconnectingPTYPort)))
conn, err := c.DialContextTCP(ctx, netip.AddrPortFrom(TailnetIP, TailnetReconnectingPTYPort))
if err != nil {
return nil, err
}
@ -199,7 +202,7 @@ func (c *AgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, w
func (c *AgentConn) SSH(ctx context.Context) (net.Conn, error) {
ctx, span := tracing.StartSpan(ctx)
defer span.End()
return c.DialContextTCP(ctx, netip.AddrPortFrom(TailnetIP, uint16(TailnetSSHPort)))
return c.DialContextTCP(ctx, netip.AddrPortFrom(TailnetIP, TailnetSSHPort))
}
// SSHClient calls SSH to create a client that uses a weak cipher
@ -226,7 +229,7 @@ func (c *AgentConn) SSHClient(ctx context.Context) (*ssh.Client, error) {
func (c *AgentConn) Speedtest(ctx context.Context, direction speedtest.Direction, duration time.Duration) ([]speedtest.Result, error) {
ctx, span := tracing.StartSpan(ctx)
defer span.End()
speedConn, err := c.DialContextTCP(ctx, netip.AddrPortFrom(TailnetIP, uint16(TailnetSpeedtestPort)))
speedConn, err := c.DialContextTCP(ctx, netip.AddrPortFrom(TailnetIP, TailnetSpeedtestPort))
if err != nil {
return nil, xerrors.Errorf("dial speedtest: %w", err)
}
@ -244,7 +247,7 @@ func (c *AgentConn) DialContext(ctx context.Context, network string, addr string
return nil, xerrors.New("network must be tcp or udp")
}
_, rawPort, _ := net.SplitHostPort(addr)
port, _ := strconv.Atoi(rawPort)
port, _ := strconv.ParseUint(rawPort, 10, 16)
ipp := netip.AddrPortFrom(TailnetIP, uint16(port))
if network == "udp" {
return c.Conn.DialContextUDP(ctx, ipp)
@ -272,7 +275,7 @@ func (c *AgentConn) statisticsClient() *http.Client {
return nil, xerrors.Errorf("request %q does not appear to be for statistics server", addr)
}
conn, err := c.DialContextTCP(context.Background(), netip.AddrPortFrom(TailnetIP, uint16(TailnetStatisticsPort)))
conn, err := c.DialContextTCP(context.Background(), netip.AddrPortFrom(TailnetIP, TailnetStatisticsPort))
if err != nil {
return nil, xerrors.Errorf("dial statistics: %w", err)
}

View File

@ -131,6 +131,9 @@ func Untar(directory string, archive []byte) error {
if err != nil {
return err
}
if header.Name == "." || strings.Contains(header.Name, "..") {
continue
}
// #nosec
target := filepath.Join(directory, filepath.FromSlash(header.Name))
switch header.Typeflag {

View File

@ -611,6 +611,9 @@ func extractBin(dest string, r io.Reader) (numExtracted int, err error) {
}
return n, xerrors.Errorf("read tar archive failed: %w", err)
}
if h.Name == "." || strings.Contains(h.Name, "..") {
continue
}
name := filepath.Join(dest, filepath.Base(h.Name))
f, err := os.Create(name)