From db4f14126e70c7cfd5df35e6393075ee0daa8652 Mon Sep 17 00:00:00 2001 From: Fernando Balieiro Ferreira Date: Tue, 21 Jan 2025 20:51:42 -0300 Subject: [PATCH] refactor(sql): refactor sql queries to use prepared statements for the resources in this commit, a the sql queries for inserting resources into the database were refactores to use prepared statements instead of simple queries this change will: - [X] add more security to the database for common attack such as sql injection --- store/db/mysql/resource.go | 50 ++++++++++++++++++++---- store/db/postgres/resource.go | 71 +++++++++++++++++++++++++++++++---- store/db/sqlite/resource.go | 58 ++++++++++++++++++++++++---- 3 files changed, 156 insertions(+), 23 deletions(-) diff --git a/store/db/mysql/resource.go b/store/db/mysql/resource.go index 00486013638df..612f22dd25421 100644 --- a/store/db/mysql/resource.go +++ b/store/db/mysql/resource.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "log/slog" "strings" "github.com/pkg/errors" @@ -30,8 +31,13 @@ func (d *DB) CreateResource(ctx context.Context, create *store.Resource) (*store } args := []any{create.UID, create.Filename, create.Blob, create.Type, create.Size, create.CreatorID, create.MemoID, storageType, create.Reference, payloadString} - stmt := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ")" - result, err := d.db.ExecContext(ctx, stmt, args...) + query := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ")" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return nil, err + } + defer stmt.Close() + result, err := stmt.ExecContext(ctx, args...) if err != nil { return nil, err } @@ -86,11 +92,21 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st } } - rows, err := d.db.QueryContext(ctx, query, args...) + stmt, err := d.db.PrepareContext(ctx, query) if err != nil { return nil, err } - defer rows.Close() + + rows, err := stmt.QueryContext(ctx, args...) + if err != nil { + return nil, err + } + defer func() { + err := stmt.Close() + if err != nil { + slog.Error("error closing statement", slog.String("message", err.Error())) + } + }() list := make([]*store.Resource, 0) for rows.Next() { @@ -138,6 +154,7 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st return list, nil } +// TODO! write a function that doesn't query everything, and instead, just one func (d *DB) GetResource(ctx context.Context, find *store.FindResource) (*store.Resource, error) { list, err := d.ListResources(ctx, find) if err != nil { @@ -177,8 +194,20 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e } args = append(args, update.ID) - stmt := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?" - result, err := d.db.ExecContext(ctx, stmt, args...) + query := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return err + } + + defer func() { + if err := stmt.Close(); err != nil { + slog.Error("error closing statement", + slog.String("message", err.Error()), + ) + } + }() + result, err := stmt.ExecContext(ctx, args...) if err != nil { return err } @@ -189,8 +218,13 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e } func (d *DB) DeleteResource(ctx context.Context, delete *store.DeleteResource) error { - stmt := "DELETE FROM `resource` WHERE `id` = ?" - result, err := d.db.ExecContext(ctx, stmt, delete.ID) + query := "DELETE FROM `resource` WHERE `id` = ?" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return err + } + defer stmt.Close() + result, err := stmt.ExecContext(ctx, delete.ID) if err != nil { return err } diff --git a/store/db/postgres/resource.go b/store/db/postgres/resource.go index e577b7aee4276..0c2b5af9846c9 100644 --- a/store/db/postgres/resource.go +++ b/store/db/postgres/resource.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "log/slog" "strings" "github.com/pkg/errors" @@ -29,8 +30,20 @@ func (d *DB) CreateResource(ctx context.Context, create *store.Resource) (*store } args := []any{create.UID, create.Filename, create.Blob, create.Type, create.Size, create.CreatorID, create.MemoID, storageType, create.Reference, payloadString} - stmt := "INSERT INTO resource (" + strings.Join(fields, ", ") + ") VALUES (" + placeholders(len(args)) + ") RETURNING id, created_ts, updated_ts" - if err := d.db.QueryRowContext(ctx, stmt, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil { + query := "INSERT INTO resource (" + strings.Join(fields, ", ") + ") VALUES (" + placeholders(len(args)) + ") RETURNING id, created_ts, updated_ts" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return nil, err + } + defer func() { + err := stmt.Close() + if err != nil { + slog.Error("failed to close statement", + slog.String("message", err.Error()), + ) + } + }() + if err = stmt.QueryRowContext(ctx, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil { return nil, err } return create, nil @@ -82,12 +95,29 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st query = fmt.Sprintf("%s OFFSET %d", query, *find.Offset) } } + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return nil, err + } - rows, err := d.db.QueryContext(ctx, query, args...) + rows, err := stmt.QueryContext(ctx, args...) if err != nil { return nil, err } - defer rows.Close() + defer func() { + err := stmt.Close() + if err != nil { + slog.Error("error closing statement", + slog.String("message", err.Error()), + ) + } + err = rows.Close() + if err != nil { + slog.Error("error closing rows", + slog.String("message", err.Error()), + ) + } + }() list := make([]*store.Resource, 0) for rows.Next() { @@ -161,9 +191,21 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e set, args = append(set, "payload = "+placeholder(len(args)+1)), append(args, string(bytes)) } - stmt := `UPDATE resource SET ` + strings.Join(set, ", ") + ` WHERE id = ` + placeholder(len(args)+1) + query := `UPDATE resource SET ` + strings.Join(set, ", ") + ` WHERE id = ` + placeholder(len(args)+1) + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return err + } + defer func() { + err := stmt.Close() + if err != nil { + slog.Error("error closing statement", + slog.String("message", err.Error()), + ) + } + }() args = append(args, update.ID) - result, err := d.db.ExecContext(ctx, stmt, args...) + result, err := stmt.ExecContext(ctx, args...) if err != nil { return err } @@ -174,8 +216,21 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e } func (d *DB) DeleteResource(ctx context.Context, delete *store.DeleteResource) error { - stmt := `DELETE FROM resource WHERE id = $1` - result, err := d.db.ExecContext(ctx, stmt, delete.ID) + query := `DELETE FROM resource WHERE id = $1` + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return err + } + defer func() { + err := stmt.Close() + if err != nil { + slog.Error("error closing statement", + slog.String("message", err.Error()), + ) + } + }() + + result, err := stmt.ExecContext(ctx, delete.ID) if err != nil { return err } diff --git a/store/db/sqlite/resource.go b/store/db/sqlite/resource.go index 6e16f28bcaa49..a634595daf0c7 100644 --- a/store/db/sqlite/resource.go +++ b/store/db/sqlite/resource.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "log/slog" "strings" "github.com/pkg/errors" @@ -30,8 +31,12 @@ func (d *DB) CreateResource(ctx context.Context, create *store.Resource) (*store } args := []any{create.UID, create.Filename, create.Blob, create.Type, create.Size, create.CreatorID, create.MemoID, storageType, create.Reference, payloadString} - stmt := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ") RETURNING `id`, `created_ts`, `updated_ts`" - if err := d.db.QueryRowContext(ctx, stmt, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil { + query := "INSERT INTO `resource` (" + strings.Join(fields, ", ") + ") VALUES (" + strings.Join(placeholder, ", ") + ") RETURNING `id`, `created_ts`, `updated_ts`" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return nil, err + } + if err := stmt.QueryRowContext(ctx, args...).Scan(&create.ID, &create.CreatedTs, &create.UpdatedTs); err != nil { return nil, err } @@ -78,12 +83,30 @@ func (d *DB) ListResources(ctx context.Context, find *store.FindResource) ([]*st query = fmt.Sprintf("%s OFFSET %d", query, *find.Offset) } } + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return nil, err + } - rows, err := d.db.QueryContext(ctx, query, args...) + rows, err := stmt.QueryContext(ctx, args...) if err != nil { return nil, err } defer rows.Close() + defer func() { + err := stmt.Close() + if err != nil { + slog.Error("failed to close statement", + slog.String("message", err.Error()), + ) + } + err = rows.Close() + if err != nil { + slog.Error("failed to close rows", + slog.String("message", err.Error()), + ) + } + }() list := make([]*store.Resource, 0) for rows.Next() { @@ -158,8 +181,18 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e } args = append(args, update.ID) - stmt := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?" - result, err := d.db.ExecContext(ctx, stmt, args...) + query := "UPDATE `resource` SET " + strings.Join(set, ", ") + " WHERE `id` = ?" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return errors.Wrap(err, "failed to prepare statement") + } + defer func() { + if err := stmt.Close(); err != nil { + slog.Error("failed to close statement", + slog.String("message", err.Error())) + } + }() + result, err := stmt.ExecContext(ctx, args...) if err != nil { return errors.Wrap(err, "failed to update resource") } @@ -170,8 +203,19 @@ func (d *DB) UpdateResource(ctx context.Context, update *store.UpdateResource) e } func (d *DB) DeleteResource(ctx context.Context, delete *store.DeleteResource) error { - stmt := "DELETE FROM `resource` WHERE `id` = ?" - result, err := d.db.ExecContext(ctx, stmt, delete.ID) + query := "DELETE FROM `resource` WHERE `id` = ?" + stmt, err := d.db.PrepareContext(ctx, query) + if err != nil { + return err + } + defer func() { + if err := stmt.Close(); err != nil { + slog.Error("failed to close statement", + slog.String("message", err.Error()), + ) + } + }() + result, err := stmt.ExecContext(ctx, stmt, delete.ID) if err != nil { return err }