Skip to content

Commit 3316f71

Browse files
adubovikovadubovikov
committed
fix: validate InfluxQL identifiers in statistic endpoints
Reject attacker-controlled database, retention, measurement and field names before they are concatenated into InfluxDB queries. Return HTTP 400 on invalid input. Bump version to 1.5.17.
1 parent 545a719 commit 3316f71

5 files changed

Lines changed: 172 additions & 6 deletions

File tree

‎controller/v1/statistic.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ func (sc *StatisticController) StatisticData(c echo.Context) error {
5454

5555
responseData, err := sc.StatisticService.StatisticData(&searchObject)
5656
if err != nil {
57-
logger.Debug(responseData)
57+
logger.Error(err.Error())
58+
return httpresponse.CreateBadResponse(&c, http.StatusBadRequest, webmessages.UserRequestFormatIncorrect)
5859
}
5960
return httpresponse.CreateSuccessResponse(&c, http.StatusCreated, responseData)
6061
}
@@ -129,7 +130,8 @@ func (sc *StatisticController) GetStatisticRetentionsList(c echo.Context) error
129130

130131
responseData, err := sc.StatisticService.StatisticRetentionsList(&searchObject)
131132
if err != nil {
132-
logger.Debug(responseData)
133+
logger.Error(err.Error())
134+
return httpresponse.CreateBadResponse(&c, http.StatusBadRequest, webmessages.UserRequestFormatIncorrect)
133135
}
134136
return httpresponse.CreateSuccessResponse(&c, http.StatusCreated, responseData)
135137
}
@@ -175,7 +177,8 @@ func (sc *StatisticController) GetStatisticMeasurementsList(c echo.Context) erro
175177

176178
responseData, err := sc.StatisticService.StatisticMeasurementsList(dbID)
177179
if err != nil {
178-
logger.Debug(responseData)
180+
logger.Error(err.Error())
181+
return httpresponse.CreateBadResponse(&c, http.StatusBadRequest, webmessages.UserRequestFormatIncorrect)
179182
}
180183
return httpresponse.CreateSuccessResponse(&c, http.StatusCreated, responseData)
181184
}
@@ -219,7 +222,8 @@ func (sc *StatisticController) GetStatisticMetricsList(c echo.Context) error {
219222

220223
responseData, err := sc.StatisticService.StatisticMetricsList(&searchObject)
221224
if err != nil {
222-
logger.Debug(responseData)
225+
logger.Error(err.Error())
226+
return httpresponse.CreateBadResponse(&c, http.StatusBadRequest, webmessages.UserRequestFormatIncorrect)
223227
}
224228
return httpresponse.CreateSuccessResponse(&c, http.StatusCreated, responseData)
225229
}
@@ -263,7 +267,8 @@ func (sc *StatisticController) GetStatisticTagsList(c echo.Context) error {
263267

264268
responseData, err := sc.StatisticService.StatisticTagsList(&searchObject)
265269
if err != nil {
266-
logger.Debug(responseData)
270+
logger.Error(err.Error())
271+
return httpresponse.CreateBadResponse(&c, http.StatusBadRequest, webmessages.UserRequestFormatIncorrect)
267272
}
268273
return httpresponse.CreateSuccessResponse(&c, http.StatusCreated, responseData)
269274
}

‎data/service/statistic.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ func (ss *StatisticService) StatisticData(statisticObject *model.StatisticObject
6161
logger.Debug("inside of the array", index)
6262
logger.Debug(query.Main)
6363

64+
if err := validateStatisticQueryEntry(query.Database, query.Retention, query.Main, query.Type); err != nil {
65+
logger.Error("Rejected statistic query parameters: ", err)
66+
return "", err
67+
}
68+
6469
counterArray := []string{}
6570
for _, el := range query.Type {
6671
meanEl := fmt.Sprintf("mean(\"%s%s\") AS %s", myPrefix, el, el)
@@ -135,6 +140,11 @@ func (ss *StatisticService) StatisticDataBaseList() (string, error) {
135140
// StatisticData: this method create new user in the database
136141
func (ss *StatisticService) StatisticRetentionsList(statisticObject *model.StatisticSearchObject) (string, error) {
137142

143+
if err := validateInfluxDatabase(statisticObject.Param.Search.Database); err != nil {
144+
logger.Error("Rejected statistic database: ", err)
145+
return "", err
146+
}
147+
138148
infQuery := fmt.Sprintf("SHOW RETENTION POLICIES ON %s", statisticObject.Param.Search.Database)
139149

140150
logger.Debug(infQuery)
@@ -168,6 +178,11 @@ func (ss *StatisticService) StatisticRetentionsList(statisticObject *model.Stati
168178
// StatisticData: this method create new user in the database
169179
func (ss *StatisticService) StatisticMeasurementsList(dbId string) (string, error) {
170180

181+
if err := validateInfluxDatabase(dbId); err != nil {
182+
logger.Error("Rejected statistic database: ", err)
183+
return "", err
184+
}
185+
171186
infQuery := fmt.Sprintf("SHOW MEASUREMENTS ON %s", dbId)
172187

173188
logger.Debug(infQuery)
@@ -201,6 +216,16 @@ func (ss *StatisticService) StatisticMeasurementsList(dbId string) (string, erro
201216
// StatisticData: this method create new user in the database
202217
func (ss *StatisticService) StatisticMetricsList(statisticObject *model.StatisticObject) (string, error) {
203218

219+
if len(statisticObject.Param.Query) == 0 {
220+
return "", fmt.Errorf("statistic query is required")
221+
}
222+
223+
query := statisticObject.Param.Query[0]
224+
if err := validateStatisticQueryEntry(query.Database, query.Retention, query.Main, nil); err != nil {
225+
logger.Error("Rejected statistic query parameters: ", err)
226+
return "", err
227+
}
228+
204229
var infQuery string
205230

206231
if statisticObject.Param.Query[0].Retention == "" || statisticObject.Param.Query[0].Retention == "none" {
@@ -239,6 +264,16 @@ func (ss *StatisticService) StatisticMetricsList(statisticObject *model.Statisti
239264
// StatisticData: this method create new user in the database
240265
func (ss *StatisticService) StatisticTagsList(statisticObject *model.StatisticObject) (string, error) {
241266

267+
if len(statisticObject.Param.Query) == 0 {
268+
return "", fmt.Errorf("statistic query is required")
269+
}
270+
271+
query := statisticObject.Param.Query[0]
272+
if err := validateStatisticQueryEntry(query.Database, query.Retention, query.Main, nil); err != nil {
273+
logger.Error("Rejected statistic query parameters: ", err)
274+
return "", err
275+
}
276+
242277
var infQuery string
243278

244279
if statisticObject.Param.Query[0].Retention == "" || statisticObject.Param.Query[0].Retention == "none" {

‎data/service/statistic_query_security.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package service
2+
3+
import (
4+
"fmt"
5+
"regexp"
6+
"strings"
7+
)
8+
9+
var (
10+
safeInfluxIdentifier = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
11+
safeInfluxMeasurement = regexp.MustCompile(`^[a-zA-Z0-9_.:/-]+$`)
12+
)
13+
14+
func validateInfluxDatabase(name string) error {
15+
if !safeInfluxIdentifier.MatchString(name) {
16+
return fmt.Errorf("invalid influx database name: %q", name)
17+
}
18+
return nil
19+
}
20+
21+
func validateInfluxRetention(name string) error {
22+
if name == "" || name == "none" {
23+
return nil
24+
}
25+
if !safeInfluxIdentifier.MatchString(name) {
26+
return fmt.Errorf("invalid influx retention policy: %q", name)
27+
}
28+
return nil
29+
}
30+
31+
func validateInfluxMeasurement(name string) error {
32+
if name == "" {
33+
return fmt.Errorf("invalid influx measurement name: empty")
34+
}
35+
if strings.ContainsAny(name, "\"\\\n\r") {
36+
return fmt.Errorf("invalid influx measurement name: %q", name)
37+
}
38+
if !safeInfluxMeasurement.MatchString(name) {
39+
return fmt.Errorf("invalid influx measurement name: %q", name)
40+
}
41+
return nil
42+
}
43+
44+
func validateInfluxFieldKey(name string) error {
45+
if !safeInfluxIdentifier.MatchString(name) {
46+
return fmt.Errorf("invalid influx field key: %q", name)
47+
}
48+
return nil
49+
}
50+
51+
func validateStatisticQueryEntry(database, retention, main string, fieldKeys []string) error {
52+
if err := validateInfluxDatabase(database); err != nil {
53+
return err
54+
}
55+
if err := validateInfluxRetention(retention); err != nil {
56+
return err
57+
}
58+
if err := validateInfluxMeasurement(main); err != nil {
59+
return err
60+
}
61+
for _, key := range fieldKeys {
62+
if err := validateInfluxFieldKey(key); err != nil {
63+
return err
64+
}
65+
}
66+
return nil
67+
}

‎data/service/statistic_query_security_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package service
2+
3+
import "testing"
4+
5+
func TestValidateInfluxDatabase_rejectsInjection(t *testing.T) {
6+
cases := []struct {
7+
name string
8+
value string
9+
valid bool
10+
}{
11+
{name: "allowed", value: "homer", valid: true},
12+
{name: "internal db", value: "_internal", valid: true},
13+
{name: "injection", value: "homer; SHOW USERS", valid: false},
14+
{name: "quote break", value: `homer" OR 1=1 --`, valid: false},
15+
}
16+
17+
for _, tc := range cases {
18+
t.Run(tc.name, func(t *testing.T) {
19+
err := validateInfluxDatabase(tc.value)
20+
if tc.valid && err != nil {
21+
t.Fatalf("expected valid database %q, got %v", tc.value, err)
22+
}
23+
if !tc.valid && err == nil {
24+
t.Fatalf("expected invalid database %q", tc.value)
25+
}
26+
})
27+
}
28+
}
29+
30+
func TestValidateInfluxMeasurement_rejectsInjection(t *testing.T) {
31+
cases := []struct {
32+
name string
33+
value string
34+
valid bool
35+
}{
36+
{name: "allowed dotted", value: "sip.calls", valid: true},
37+
{name: "quote break", value: `cpu" OR 1=1 --`, valid: false},
38+
{name: "semicolon", value: "cpu; SHOW USERS", valid: false},
39+
}
40+
41+
for _, tc := range cases {
42+
t.Run(tc.name, func(t *testing.T) {
43+
err := validateInfluxMeasurement(tc.value)
44+
if tc.valid && err != nil {
45+
t.Fatalf("expected valid measurement %q, got %v", tc.value, err)
46+
}
47+
if !tc.valid && err == nil {
48+
t.Fatalf("expected invalid measurement %q", tc.value)
49+
}
50+
})
51+
}
52+
}
53+
54+
func TestValidateStatisticQueryEntry_rejectsBadFieldKey(t *testing.T) {
55+
err := validateStatisticQueryEntry("homer", "autogen", "cpu", []string{`value") AS x --`})
56+
if err == nil {
57+
t.Fatal("expected invalid field key to be rejected")
58+
}
59+
}

‎version.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package main
22

33
// VERSION
4-
var VERSION_APPLICATION = "1.5.16"
4+
var VERSION_APPLICATION = "1.5.17"
55

66
// NAME
77
var NAME_APPLICATION = "homer-app"

0 commit comments

Comments
 (0)