Prepared statements are expensive.

[lithdew]

kgs/app/adapter/db/allocatedkey.go

Lines 28 to 31 in 7d538f0

	statement, err := tx.Prepare(
	fmt.Sprintf(`
	INSERT INTO "%s" ("%s", "%s")
	VALUES ($1, $2);

Prepared statements should only be prepared once during initialization as they are expensive to make. They should be cleaned up upon graceful shutdown of the application.

Also, for best practices, do not use fmt.Sprintf in any manner for formatting out SQL queries (there are many ways this may expose the ability for one to perform SQL injection). Provide table names and column names as typed parameters that are to be escaped by the query engine.

[magicoder10]

@lithdew I thought parameterized queries are only vulnerable if the hacker can inject in the variables.

[magicoder10]

@lithdew Will I be wasting resources if I never use the prepared the statement?

[magicoder10]

@lithdew Thank you so much for the suggestions! I enjoyed those kinds of technical discussion.

[lithdew]

@lithdew I thought parameterized queries are only vulnerable if the hacker can inject in the variables.

Misconfiguration can also lead to possible injection of variables.

[lithdew]

@lithdew Will I be wasting resources if I never use the prepared the statement?

A minimal amount: it's better to only instantiate a prepared statement once throughout an application lifecycle.

[oatovar]

@lithdew I thought parameterized queries are only vulnerable if the hacker can inject in the variables.

Misconfiguration can also lead to possible injection of variables.

Can you elaborate more on a scenario where this could be introduced? At first glance the source of the input, would have to be updated and merged into the repo where code review should catch it.

[magicoder10]

@lithdew I see. Are You trying to avoid people from accidentally passing params into fmt?

[magicoder10]

@oatovar Can be problematic if the code reviewer is reviewing the code at 3 AM in the early morning.

[magicoder10]

@lithdew From my research, prepare will send the query to the db ahead of time and stream the data later on to prevent SQL injection.

[lithdew]

@lithdew From my research, prepare will send the query to the db ahead of time and stream the data later on to prevent SQL injection.

Right, this is why instantiating a prepared statement should ideally only be done once. Otherwise, there is no benefit to using prepared statements.

The issue of SQL injection is that should an attacker be able to pass in any parameters for either the table or table column names, they would be able to pass in arbitrary SQL queries.

Instantiating the prepared statement once on startup is a good countermeasure for this, since the prepared statement won't change during runtime. On top of that, it is just best practice to not use string formatting methods like fmt.Sprintf to lay out sensitive strings like SQL statements. You should take advantage of the underlying SQL engine's ability to 'escape' parameters provided to execute a SQL statement using named/anonymous parameters ('?', '$1').