Skip to content

Commit

Permalink
Merge commit from fork
Browse files Browse the repository at this point in the history 
These templates were rendered using text/template which is fundamentally
broken as it would allow for trivial HTML injection.

Instead render using safehtml/template so that we have automatic
escaping.

Signed-off-by: Dirkjan Bussink <[email protected]>
  • Loading branch information
@dbussink
dbussink authored Dec 2, 2024
1 parent 7bb2369 commit b56b717
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 13 deletions.
3 changes: 2 additions & 1 deletion go/vt/vtgate/debugenv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ import (
"html"
"net/http"
"strconv"
"text/template"
"time"

"github.com/google/safehtml/template"

"vitess.io/vitess/go/acl"
"vitess.io/vitess/go/vt/discovery"
"vitess.io/vitess/go/vt/log"
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/querylogz.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ import (
"net/http"
"strconv"
"strings"
"text/template"
"time"

"vitess.io/vitess/go/vt/vtgate/logstats"
"github.com/google/safehtml/template"

"vitess.io/vitess/go/acl"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/logz"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/vtgate/logstats"
)

var (
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtgate/querylogz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (

func TestQuerylogzHandlerFormatting(t *testing.T) {
req, _ := http.NewRequest("GET", "/querylogz?timeout=10&limit=1", nil)
logStats := logstats.NewLogStats(context.Background(), "Execute", "select name from test_table limit 1000", "suuid", nil)
logStats := logstats.NewLogStats(context.Background(), "Execute", "select name, 'inject <script>alert();</script>' from test_table limit 1000", "suuid", nil)
logStats.StmtType = "select"
logStats.RowsAffected = 1000
logStats.ShardQueries = 1
Expand Down Expand Up @@ -64,7 +64,7 @@ func TestQuerylogzHandlerFormatting(t *testing.T) {
`<td>0.002</td>`,
`<td>0.003</td>`,
`<td>select</td>`,
`<td>select name from test_table limit 1000</td>`,
regexp.QuoteMeta(`<td>select name,​ &#39;inject &lt;script&gt;alert()​;&lt;/script&gt;&#39; from test_table limit 1000</td>`),
`<td>1</td>`,
`<td>1000</td>`,
`<td></td>`,
Expand Down Expand Up @@ -94,7 +94,7 @@ func TestQuerylogzHandlerFormatting(t *testing.T) {
`<td>0.002</td>`,
`<td>0.003</td>`,
`<td>select</td>`,
`<td>select name from test_table limit 1000</td>`,
regexp.QuoteMeta(`<td>select name,​ &#39;inject &lt;script&gt;alert()​;&lt;/script&gt;&#39; from test_table limit 1000</td>`),
`<td>1</td>`,
`<td>1000</td>`,
`<td></td>`,
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestQuerylogzHandlerFormatting(t *testing.T) {
`<td>0.002</td>`,
`<td>0.003</td>`,
`<td>select</td>`,
`<td>select name from test_table limit 1000</td>`,
regexp.QuoteMeta(`<td>select name,​ &#39;inject &lt;script&gt;alert()​;&lt;/script&gt;&#39; from test_table limit 1000</td>`),
`<td>1</td>`,
`<td>1000</td>`,
`<td></td>`,
Expand Down
3 changes: 2 additions & 1 deletion go/vt/vttablet/tabletserver/debugenv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ import (
"html"
"net/http"
"strconv"
"text/template"
"time"

"github.com/google/safehtml/template"

"vitess.io/vitess/go/acl"
"vitess.io/vitess/go/vt/log"
)
Expand Down
3 changes: 2 additions & 1 deletion go/vt/vttablet/tabletserver/querylogz.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ import (
"net/http"
"strconv"
"strings"
"text/template"
"time"

"github.com/google/safehtml/template"

"vitess.io/vitess/go/acl"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/logz"
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vttablet/tabletserver/querylogz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestQuerylogzHandler(t *testing.T) {
req, _ := http.NewRequest("GET", "/querylogz?timeout=10&limit=1", nil)
logStats := tabletenv.NewLogStats(context.Background(), "Execute")
logStats.PlanType = planbuilder.PlanSelect.String()
logStats.OriginalSQL = "select name from test_table limit 1000"
logStats.OriginalSQL = "select name, 'inject <script>alert();</script>' from test_table limit 1000"
logStats.RowsAffected = 1000
logStats.NumberOfQueries = 1
logStats.StartTime, _ = time.Parse("Jan 2 15:04:05", "Nov 29 13:33:09")
Expand All @@ -64,7 +64,7 @@ func TestQuerylogzHandler(t *testing.T) {
`<td>0.001</td>`,
`<td>1e-08</td>`,
`<td>Select</td>`,
`<td>select name from test_table limit 1000</td>`,
regexp.QuoteMeta(`<td>select name,​ &#39;inject &lt;script&gt;alert()​;&lt;/script&gt;&#39; from test_table limit 1000</td>`),
`<td>1</td>`,
`<td>none</td>`,
`<td>1000</td>`,
Expand Down Expand Up @@ -95,7 +95,7 @@ func TestQuerylogzHandler(t *testing.T) {
`<td>0.001</td>`,
`<td>1e-08</td>`,
`<td>Select</td>`,
`<td>select name from test_table limit 1000</td>`,
regexp.QuoteMeta(`<td>select name,​ &#39;inject &lt;script&gt;alert()​;&lt;/script&gt;&#39; from test_table limit 1000</td>`),
`<td>1</td>`,
`<td>none</td>`,
`<td>1000</td>`,
Expand Down Expand Up @@ -126,7 +126,7 @@ func TestQuerylogzHandler(t *testing.T) {
`<td>0.001</td>`,
`<td>1e-08</td>`,
`<td>Select</td>`,
`<td>select name from test_table limit 1000</td>`,
regexp.QuoteMeta(`<td>select name,​ &#39;inject &lt;script&gt;alert()​;&lt;/script&gt;&#39; from test_table limit 1000</td>`),
`<td>1</td>`,
`<td>none</td>`,
`<td>1000</td>`,
Expand Down

0 comments on commit b56b717

Please sign in to comment.