From f6c859061bfd7ccc2a21fcde3e9f0eb9ad98cd5e Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Sun, 30 Jan 2022 10:17:36 -0700 Subject: [PATCH] Fix CodeQL warnings --- .github/workflows/integration_test.yml | 8 ++++---- db.go | 8 ++++++-- integration/replica_client_test.go | 12 +++++++----- internal/internal.go | 20 +++++++++++++++----- s3/replica_client.go | 8 ++++---- sftp/replica_client.go | 21 ++++++++++++++++++--- 6 files changed, 54 insertions(+), 23 deletions(-) diff --git a/.github/workflows/integration_test.yml b/.github/workflows/integration_test.yml index b947afb..afeccca 100644 --- a/.github/workflows/integration_test.yml +++ b/.github/workflows/integration_test.yml @@ -101,10 +101,10 @@ jobs: - name: Run sftp tests w/ key run: go test -v -run=TestReplicaClient ./integration -replica-type sftp env: - LITESTREAM_SFTP_HOST: litestream-test-sftp.fly.dev:2222 - LITESTREAM_SFTP_USER: litestream - LITESTREAM_SFTP_KEY_PATH: /opt/id_ed25519 - LITESTREAM_SFTP_PATH: /litestream + LITESTREAM_SFTP_HOST: litestream-test-sftp.fly.dev:2222 + LITESTREAM_SFTP_USER: litestream + LITESTREAM_SFTP_PATH: /litestream + LITESTREAM_SFTP_KEY_PATH: /opt/id_ed25519 - name: Run sftp tests w/ password run: go test -v -run=TestReplicaClient ./integration -replica-type sftp diff --git a/db.go b/db.go index 49fbf21..85d146d 100644 --- a/db.go +++ b/db.go @@ -12,6 +12,7 @@ import ( "io" "io/ioutil" "log" + "math" "math/rand" "os" "path/filepath" @@ -1593,8 +1594,11 @@ func parseWALPath(s string) (index int, err error) { return 0, fmt.Errorf("invalid wal path: %s", s) } - i64, _ := strconv.ParseUint(a[1], 16, 64) - return int(i64), nil + i32, _ := strconv.ParseUint(a[1], 16, 32) + if i32 > math.MaxInt32 { + return 0, fmt.Errorf("index too large in wal path: %s", s) + } + return int(i32), nil } // formatWALPath formats a WAL filename with a given index. diff --git a/integration/replica_client_test.go b/integration/replica_client_test.go index 109f4f3..8761965 100644 --- a/integration/replica_client_test.go +++ b/integration/replica_client_test.go @@ -59,11 +59,12 @@ var ( // SFTP settings var ( - sftpHost = flag.String("sftp-host", os.Getenv("LITESTREAM_SFTP_HOST"), "") - sftpUser = flag.String("sftp-user", os.Getenv("LITESTREAM_SFTP_USER"), "") - sftpPassword = flag.String("sftp-password", os.Getenv("LITESTREAM_SFTP_PASSWORD"), "") - sftpKeyPath = flag.String("sftp-key-path", os.Getenv("LITESTREAM_SFTP_KEY_PATH"), "") - sftpPath = flag.String("sftp-path", os.Getenv("LITESTREAM_SFTP_PATH"), "") + sftpHost = flag.String("sftp-host", os.Getenv("LITESTREAM_SFTP_HOST"), "") + sftpUser = flag.String("sftp-user", os.Getenv("LITESTREAM_SFTP_USER"), "") + sftpPassword = flag.String("sftp-password", os.Getenv("LITESTREAM_SFTP_PASSWORD"), "") + sftpKeyPath = flag.String("sftp-key-path", os.Getenv("LITESTREAM_SFTP_KEY_PATH"), "") + sftpHostKeyPath = flag.String("sftp-host-key-path", os.Getenv("LITESTREAM_SFTP_HOST_KEY_PATH"), "") + sftpPath = flag.String("sftp-path", os.Getenv("LITESTREAM_SFTP_PATH"), "") ) func TestReplicaClient_Generations(t *testing.T) { @@ -538,6 +539,7 @@ func NewSFTPReplicaClient(tb testing.TB) *sftp.ReplicaClient { c.User = *sftpUser c.Password = *sftpPassword c.KeyPath = *sftpKeyPath + c.HostKeyPath = *sftpHostKeyPath c.Path = path.Join(*sftpPath, fmt.Sprintf("%016x", rand.Uint64())) return c } diff --git a/internal/internal.go b/internal/internal.go index 36598d4..cb23a34 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -3,6 +3,7 @@ package internal import ( "fmt" "io" + "math" "os" "regexp" "strconv" @@ -159,8 +160,11 @@ func ParseSnapshotPath(s string) (index int, err error) { return 0, fmt.Errorf("invalid snapshot path") } - i64, _ := strconv.ParseUint(a[1], 16, 64) - return int(i64), nil + i32, _ := strconv.ParseUint(a[1], 16, 32) + if i32 > math.MaxInt32 { + return 0, fmt.Errorf("index too large in snapshot path %q", s) + } + return int(i32), nil } var snapshotPathRegex = regexp.MustCompile(`^([0-9a-f]{8})\.snapshot\.lz4$`) @@ -172,9 +176,15 @@ func ParseWALSegmentPath(s string) (index int, offset int64, err error) { return 0, 0, fmt.Errorf("invalid wal segment path") } - i64, _ := strconv.ParseUint(a[1], 16, 64) - off64, _ := strconv.ParseUint(a[2], 16, 64) - return int(i64), int64(off64), nil + i32, _ := strconv.ParseUint(a[1], 16, 32) + if i32 > math.MaxInt32 { + return 0, 0, fmt.Errorf("index too large in wal segment path %q", s) + } + off64, _ := strconv.ParseInt(a[2], 16, 64) + if off64 > math.MaxInt64 { + return 0, 0, fmt.Errorf("offset too large in wal segment path %q", s) + } + return int(i32), int64(off64), nil } var walSegmentPathRegex = regexp.MustCompile(`^([0-9a-f]{8})\/([0-9a-f]{8})\.wal\.lz4$`) diff --git a/s3/replica_client.go b/s3/replica_client.go index a739a5f..a9e3e63 100644 --- a/s3/replica_client.go +++ b/s3/replica_client.go @@ -715,10 +715,10 @@ func ParseHost(s string) (bucket, region, endpoint string, forcePathStyle bool) var ( localhostRegex = regexp.MustCompile(`^(?:(.+)\.)?localhost$`) - backblazeRegex = regexp.MustCompile(`^(?:(.+)\.)?s3.([^.]+)\.backblazeb2.com$`) - filebaseRegex = regexp.MustCompile(`^(?:(.+)\.)?s3.filebase.com$`) - digitalOceanRegex = regexp.MustCompile(`^(?:(.+)\.)?([^.]+)\.digitaloceanspaces.com$`) - linodeRegex = regexp.MustCompile(`^(?:(.+)\.)?([^.]+)\.linodeobjects.com$`) + backblazeRegex = regexp.MustCompile(`^(?:(.+)\.)?s3\.([^.]+)\.backblazeb2\.com$`) + filebaseRegex = regexp.MustCompile(`^(?:(.+)\.)?s3\.filebase\.com$`) + digitalOceanRegex = regexp.MustCompile(`^(?:(.+)\.)?([^.]+)\.digitaloceanspaces\.com$`) + linodeRegex = regexp.MustCompile(`^(?:(.+)\.)?([^.]+)\.linodeobjects\.com$`) ) func isNotExists(err error) bool { diff --git a/sftp/replica_client.go b/sftp/replica_client.go index 30d8fa8..8b651e9 100644 --- a/sftp/replica_client.go +++ b/sftp/replica_client.go @@ -41,6 +41,7 @@ type ReplicaClient struct { Password string Path string KeyPath string + HostKeyPath string DialTimeout time.Duration } @@ -71,14 +72,28 @@ func (c *ReplicaClient) Init(ctx context.Context) (_ *sftp.Client, err error) { // Build SSH configuration & auth methods config := &ssh.ClientConfig{ - User: c.User, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), - BannerCallback: ssh.BannerDisplayStderr(), + User: c.User, + BannerCallback: ssh.BannerDisplayStderr(), } if c.Password != "" { config.Auth = append(config.Auth, ssh.Password(c.Password)) } + if c.HostKeyPath == "" { + config.HostKeyCallback = ssh.InsecureIgnoreHostKey() + } else { + buf, err := os.ReadFile(c.HostKeyPath) + if err != nil { + return nil, fmt.Errorf("cannot read sftp host key path: %w", err) + } + + key, _, _, _, err := ssh.ParseAuthorizedKey(buf) + if err != nil { + return nil, fmt.Errorf("cannot parse sftp host key path: path=%s len=%d err=%w", c.HostKeyPath, len(buf), err) + } + config.HostKeyCallback = ssh.FixedHostKey(key) + } + if c.KeyPath != "" { buf, err := os.ReadFile(c.KeyPath) if err != nil {