SQL Injection

[jfyne]

Hi there,

I really like the look of this project, and it could be very helpful for me. However does the approach you are taking protect against any sort of SQL injection?

Generally I would expect the SQL statement to be defined as such, notice the ? instead of {{ .Id }}

{{ define "SelectBlog" }}
select
        id,
        title,
        author_id
from blog
where id = ? limit 1
{{ end }}

Then the Id argument to be passed into the query:

    rows, err := m.DB.Query(rawSql, r.Id)
    defer rows.Close()
        ...

It looks as though this is not happening and raw input is just being passed straight into the query. Have you thought about a way to prevent an attack?

Or am I just missing something?

Thanks for the work!

[jackskj]

Very good point and you would be correct. Templating makes parameterised queries a little tricky, especially with if statements or for loops in the template. I think the best solution is to let the developer indicate which inputs are to be parametrized. This can be done with a template helper function such as:

{{ define "SelectBlog" }}
select id, title, author_id from blog 
where id =   {{ .Id | param }} 
{{ end }}

this would create the following query

select id, title, author_id from blog 
where id ?

which would be prepared by database/sql. The reason I would go with a "param" function is because a field in your request message may not be an input to the query, but rather an augmentation to the query itself. Here is a simple example:

{{ define "SelectBlog" }}
select id, title, author_id from blog 
    {{ if .OrderBy }} 
    order by id
    {{ end }}
{{ end }}

Let me know your opinion.

[jfyne]

I do like the templating approach and the logic that it allows.

And in this scenario the param function would gather the correct set of arguments to pass to the query in order? I do wonder about how the ordering of the function would be called on executing the template

Overall sounds like a reasonable approach to me.

[jackskj]

I would take care of the ordering. It's more about whether this solution is valid. I'll take care of it. Thanks for comment.

[jackskj]

release v.0.4.0 resolves this issue