1

I'm trying to create query helpers for my repository pattern. Actually I'm dealing how to add join clauses. The only idea is to inject Sprintf statement but this behavior might cause SQL injection.

Here's my query builder

func getQueryOptions(q *gorm.DB, opts ...query.QueryOption) *gorm.DB {
    options := query.NewQueryOptions(opts...)

    if joins := options.Joins; joins != nil {
        for _, join := range joins {
            q = q.Clauses(clause.Join{
                Type: clause.JoinType(join.Type),
                Table: clause.Table{
                    Name:  join.Table.Name,
                    Alias: join.Table.Alias,
                },
                ON: clause.Where{Exprs: []clause.Expression{
                    clause.Expr{SQL: join.On, Vars: join.Args},
                }},
            })
        }
    }
if filters := options.Filters; filters != nil {
        for _, filter := range filters {
            q = q.Where(filter.Condition, filter.Args...)
        }
    }
 [...]
}

here's my test

dbGames, err := repo.FindAll(
                    query.WithJoin(query.JoinType.InnerJoin, "platforms_games", "pg", "pg.game_id = games.id"),
                    query.WithJoin(query.JoinType.InnerJoin, "platforms", "", "pg.platform_id = platforms.id"),
                    query.WithFilter("platforms.name = ?", "Platform 1"),
                )
                assert.NoError(t, err)
                assert.Len(t, dbGames, 1)

but it returns incorrect SQL query

SELECT * FROM "games" WHERE INNER JOIN "platforms_games" "pg" ON pg.game_id = games.id AND INNER JOIN "platforms" ON pg.platform_id = platforms.id AND platforms.name = 'Platform 1'
5
  • 1
    You can mitigate SQL injection risks by formatting queries only with content that is fixed and/or under control of your application. Like variables or lists that are set literally in your code. Don't concatenate untrusted content into SQL queries. Commented Aug 29 at 22:56
  • @DaleK I would suggest to read the title. Commented Aug 30 at 16:57
  • @BillKarwin I know that but I thought GORM was mature enough that I wouldn't have to “play around” with string concatenation. thanks anyway Commented Aug 30 at 16:57
  • You need to repeat the question after you explain everything, because that's when it makes sense Commented Aug 30 at 19:31
  • Where exactly do you have to use Sprintf? It's not in the question' example code. The only issue I see with resulting SQL query is incorrectly placed WHERE condition, if you want to address it with current code (without Sprintf) just say so? Commented Aug 30 at 21:19

1 Answer 1

1

That happens because in your builder you’re using q.Clauses(clause.Join{ ... }), but by default GORM interprets it as a WHERE clause unless you wrap it properly in clause.Join.
try this, Instead of injecting raw SQL with Sprintf, stick with GORM’s clause API. For example:

q = q.Clauses(clause.Join{
    Type: clause.JoinType(join.Type),
    Table: clause.Table{
        Name:  join.Table.Name,
        Alias: join.Table.Alias,
    },
    ON: clause.Where{
        Exprs: []clause.Expression{
            clause.Eq{Column: clause.Column{Table: "pg", Name: "game_id"}, Value: clause.Column{Table: "games", Name: "id"}},
        },
    },
})

This way GORM knows it’s a JOIN clause, not a WHERE.

Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.