github-code-scanning[bot] commented on code in PR #11782:
URL: https://github.com/apache/dolphinscheduler/pull/11782#discussion_r967831967
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -1047,6 +1090,7 @@
}
// check file content
if (CollectionUtils.isEmpty(dagDataScheduleList)) {
+ logger.warn("Process definition file content is null,
fileName:{}.", file.getOriginalFilename());
Review Comment:
## Log Injection
This user-provided value flows to a [log entry](1).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1031)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -1188,7 +1233,7 @@
}
}
} catch (Exception e) {
- logger.error(e.getMessage(), e);
+ logger.error("Import process definition error, projectCode:{},
fileName:{}.", projectCode, file.getOriginalFilename(), e);
Review Comment:
## Log Injection
This user-provided value flows to a [log entry](1).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1033)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProcessDefinitionServiceImpl.java:
##########
@@ -1726,16 +1773,10 @@
~ treeViewDto.setCode(taskNode.getCode());
~ treeViewDto.setName(taskNode.getName());
// set treeViewDto instances
for (int i = limit - 1; i >= 0; i--) {
ProcessInstance processInstance =
processInstanceList.get(i);
- TaskInstance taskInstance = null;
- for (TaskInstance instance : taskInstances) {
- if (instance.getTaskCode() == Long.parseLong(nodeCode)
- && instance.getProcessInstanceId() ==
processInstance.getId()) {
- taskInstance = instance;
- break;
- }
- }
+ TaskInstance taskInstance =
taskInstanceMapper.queryByInstanceIdAndCode(processInstance.getId(),
+ Long.parseLong(nodeCode));
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1030)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java:
##########
@@ -592,21 +606,21 @@
putMsg(result, Status.SUCCESS);
if (FileUtils.directoryTraversal(name)) {
- logger.error("file alias name {} verify failed", name);
+ logger.warn("Parameter file alias name verify failed,
fileAliasName:{}.", name);
putMsg(result, Status.VERIFY_PARAMETER_NAME_FAILED);
return result;
}
if (file != null &&
FileUtils.directoryTraversal(Objects.requireNonNull(file.getOriginalFilename())))
{
- logger.error("file original name {} verify failed",
file.getOriginalFilename());
+ logger.warn("File original name verify failed,
fileOriginalName:{}.", file.getOriginalFilename());
Review Comment:
## Log Injection
This user-provided value flows to a [log entry](1).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1034)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ExecutorController.java:
##########
@@ -294,7 +298,9 @@
public Result execute(@ApiIgnore @RequestAttribute(value =
Constants.SESSION_USER) User loginUser,
@ApiParam(name = "projectCode", value =
"PROJECT_CODE", required = true) @PathVariable long projectCode,
@RequestParam("processInstanceId") Integer
processInstanceId,
- @RequestParam("executeType") ExecuteType
executeType) {
+ @RequestParam("executeType") ExecuteType executeType
Review Comment:
## Log Injection
This user-provided value flows to a [log entry](1).
This user-provided value flows to a [log entry](2).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1053)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/permission/PermissionCheck.java:
##########
@@ -168,14 +168,14 @@
// get user type in order to judge whether the user is admin
User user = processService.getUserById(userId);
if (user == null) {
- logger.error("user id {} doesn't exist", userId);
+ logger.error("User does not exist, userId:{}.", userId);
throw new ServiceException(String.format("user %s doesn't
exist", userId));
}
if (user.getUserType() != UserType.ADMIN_USER) {
List<T> unauthorizedList =
processService.listUnauthorized(userId, needChecks, authorizationType);
// if exist unauthorized resource
if (CollectionUtils.isNotEmpty(unauthorizedList)) {
- logger.error("user {} doesn't have permission of {}: {}",
user.getUserName(), authorizationType.getDescp(), unauthorizedList);
+ logger.error("User does not have permission for {}: {}",
user.getUserName(), authorizationType.getDescp(), unauthorizedList);
Review Comment:
## Unused format argument
This format call refers to 2 argument(s) but supplies 3 argument(s).
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1027)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java:
##########
@@ -533,7 +556,7 @@
return Constants.READ_PERMISSION;
}
- if (Objects.equals(project.getUserId(), user.getId())) {
+ if (project.getUserId() == user.getId()) {
Review Comment:
## Reference equality test of boxed types
Suspicious reference comparison of boxed numerical values.
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1024)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/permission/ResourcePermissionCheckServiceImpl.java:
##########
@@ -107,6 +107,8 @@
Set<?> originResSet = new HashSet<>(Arrays.asList(needChecks));
Set<?> ownResSets =
RESOURCE_LIST_MAP.get(authorizationType).listAuthorizedResource(userId, logger);
originResSet.removeAll(ownResSets);
+ if (CollectionUtils.isNotEmpty(originResSet))
+ logger.warn("User does not have resource permission {} on
associated resources, userId:{}", authorizationType, userId);
Review Comment:
## Insertion of sensitive information into log files
This [potentially sensitive information](1) is written to a log file.
This [potentially sensitive information](2) is written to a log file.
This [potentially sensitive information](3) is written to a log file.
This [potentially sensitive information](4) is written to a log file.
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1116)
##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java:
##########
@@ -1084,16 +1140,16 @@
String msg = null;
if (!CheckUtils.checkUserName(userName)) {
-
+ logger.warn("Parameter userName check failed, userName:{}.",
userName);
msg = userName;
} else if (!CheckUtils.checkPassword(password)) {
-
+ logger.warn("Parameter password check failed, password:{}.",
password);
Review Comment:
## Insertion of sensitive information into log files
This [potentially sensitive information](1) is written to a log file.
[Show more
details](https://github.com/apache/dolphinscheduler/security/code-scanning/1117)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
