You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/06/20 17:51:44 UTC
[GitHub] [skywalking] Jumbo-WJB opened a new issue #4955: ALARM_MESSAGE Sql Inject
Jumbo-WJB opened a new issue #4955:
URL: https://github.com/apache/skywalking/issues/4955
ALARM_MESSAGE Sql Inject
file path:
https://github.com/apache/skywalking/blob/master/oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/mysql/MySQLAlarmQueryDAO.java#L63
The parameters are spliced directly into the sql query, causing sql injection：
request:
```
POST /graphql HTTP/1.1
Host: host:8080
Content-Length: 479
Accept: application/json, text/plain, */*
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.106 Safari/537.36
Content-Type: application/json;charset=UTF-8
Origin: http://host:8080
Referer: http://host:8080/alarm
Accept-Encoding: gzip, deflate
Accept-Language: zh-CN,zh;q=0.9,en;q=0.8
Connection: close
{"query":"query queryAlarms($keyword: String, $scope: Scope, $duration:Duration!, $paging: Pagination!) {\n getAlarm(keyword: $keyword, scope: $scope, duration: $duration, paging: $paging) {\n items: msgs {\n key: id\n message\n startTime\n scope\n }\n total\n }}","variables":{"duration":{"start":"2020-06-20 2323","end":"2020-06-20 2338","step":"MINUTE"},"paging":{"pageNum":1,"pageSize":20,"needTotal":true},"keyword":"1111'"}}
```
response:
```
HTTP/1.1 200
X-Application-Context: application:8080
Date: Sat, 20 Jun 2020 17:50:18 GMT
Content-Type: application/json;charset=utf-8
Connection: close
Content-Length: 488
{"data":{},"errors":[{"message":"Exception while fetching data (/getAlarm) : Syntax error in SQL statement \"select count(1) total from (select 1 from alarm_record where 1=1 and time_bucket >= ? and time_bucket <= ? and alarm_message like '%1111'%[*]' order by start_time desc )\"; SQL statement:\nselect count(1) total from (select 1 from alarm_record where 1=1 and time_bucket >= ? and time_bucket <= ? and alarm_message like '%1111'%' order by start_time desc ) [42000-196]"}]}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] JaredTan95 closed issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
JaredTan95 closed issue #4955:
URL: https://github.com/apache/skywalking/issues/4955
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] neargle edited a comment on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
neargle edited a comment on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647140092
@wu-sheng Hi.
It is better not to fix only the single point in MySQLAlarmQueryDAO.java#L63 here, it seems that there are many codes using coding methods that maybe at risk of SQL injection.
It is possible to have SQL injection problems when appending a string (or other variable that can be coerced into string type) that can be controlled by an user to the SQL statement for direct sql query.
Using CodeQL and grep to filter a part of them, the following is a possible list:
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2AggregationQueryDAO.java
additionalConditions.forEach(condition -> {
sql.append(" and ").append(condition.getKey()).append("=?");
conditions.add(condition.getValue());
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2AlarmQueryDAO.java
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(AlarmRecord.ALARM_MESSAGE).append(" like '%").append(keyword).append("%' ");
}
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2LogQueryDAO.java
sql.append("from ").append(metricName).append(" where ");
sql.append(" 1=1 ");
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2MetadataQueryDAO.java
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(ServiceTraffic.NAME).append(" like \"%").append(keyword).append("%\"");
}
sql.append(" limit ").append(metadataQueryMaxSize);
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(EndpointTraffic.NAME).append(" like '%").append(keyword).append("%' ");
}
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2TopNRecordsQueryDAO.java
}
sql.append(" limit ").append(condition.getTopN());
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/mysql/MySQLAlarmQueryDAO.java
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(AlarmRecord.ALARM_MESSAGE).append(" like '%").append(keyword).append("%' ");
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647057292
Any interest to help on fixing this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647141347
You could send a pull request to fix all these directly. We prefer our contributors could do this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647141430
Also, as MySQL is not 1st priority/recommended storage in the product env, we want to encourage people to join us.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647069963
@Jumbo-WJB I know how to fix. My question is, should we wait for your pull request?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] Jumbo-WJB commented on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
Jumbo-WJB commented on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647062960
of course，you can refer to https://github.com/apache/skywalking/pull/4639 to fix this vuln.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] neargle commented on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
neargle commented on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647140092
It is better not to fix only the single point in MySQLAlarmQueryDAO.java#L63 here, it seems that there are many codes using coding methods that maybe at risk of SQL injection.
It is possible to have SQL injection problems when appending a string (or other variable that can be coerced into string type) that can be controlled by an user to the SQL statement for direct sql query.
Using CodeQL and grep to filter a part of them, the following is a possible list:
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2AggregationQueryDAO.java
additionalConditions.forEach(condition -> {
sql.append(" and ").append(condition.getKey()).append("=?");
conditions.add(condition.getValue());
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2AlarmQueryDAO.java
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(AlarmRecord.ALARM_MESSAGE).append(" like '%").append(keyword).append("%' ");
}
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2LogQueryDAO.java
sql.append("from ").append(metricName).append(" where ");
sql.append(" 1=1 ");
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2MetadataQueryDAO.java
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(ServiceTraffic.NAME).append(" like \"%").append(keyword).append("%\"");
}
sql.append(" limit ").append(metadataQueryMaxSize);
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(EndpointTraffic.NAME).append(" like '%").append(keyword).append("%' ");
}
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2TopNRecordsQueryDAO.java
}
sql.append(" limit ").append(condition.getTopN());
oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/mysql/MySQLAlarmQueryDAO.java
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(AlarmRecord.ALARM_MESSAGE).append(" like '%").append(keyword).append("%' ");
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] neargle edited a comment on issue #4955: ALARM_MESSAGE Sql Inject
Posted by GitBox <gi...@apache.org>.
neargle edited a comment on issue #4955:
URL: https://github.com/apache/skywalking/issues/4955#issuecomment-647140092
@wu-sheng Hi.
It is better not to fix only the single point in MySQLAlarmQueryDAO.java#L63 here, it seems that there are many codes using coding methods that maybe at risk of SQL injection.
It is possible to have SQL injection problems when appending a string (or other variable that can be coerced into string type) that can be controlled by an user to the SQL statement for direct sql query.
Using CodeQL and grep to filter a part of them, the following is a possible list:
**oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2AggregationQueryDAO.java**
additionalConditions.forEach(condition -> {
sql.append(" and ").append(condition.getKey()).append("=?");
conditions.add(condition.getValue());
**oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2AlarmQueryDAO.java**
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(AlarmRecord.ALARM_MESSAGE).append(" like '%").append(keyword).append("%' ");
}
**oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2LogQueryDAO.java**
sql.append("from ").append(metricName).append(" where ");
sql.append(" 1=1 ");
**oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2MetadataQueryDAO.java**
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(ServiceTraffic.NAME).append(" like \"%").append(keyword).append("%\"");
}
sql.append(" limit ").append(metadataQueryMaxSize);
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(EndpointTraffic.NAME).append(" like '%").append(keyword).append("%' ");
}
**oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2TopNRecordsQueryDAO.java**
}
sql.append(" limit ").append(condition.getTopN());
**oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/mysql/MySQLAlarmQueryDAO.java**
if (!Strings.isNullOrEmpty(keyword)) {
sql.append(" and ").append(AlarmRecord.ALARM_MESSAGE).append(" like '%").append(keyword).append("%' ");
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org