Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/main/java/org/joychou/controller/CommandInject2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package org.joychou.controller;

import org.joychou.security.SecurityUtil;
import org.joychou.util.WebUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

import javax.servlet.http.HttpServletRequest;
import java.io.IOException;

@RestController
public class CommandInject {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

类名与文件名不匹配,应当保持一致

文件名为CommandInject2.java,但类名为CommandInject。应确保类名与文件名一致,避免混淆。

- public class CommandInject {
+ public class CommandInject2 {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class CommandInject {
public class CommandInject2 {


protected final Logger logger = LoggerFactory.getLogger(this.getClass());

/**
* http://localhost:8080/codeinject?filepath=/tmp;cat /etc/passwd
*
* @param filepath filepath
* @return result
*/
@GetMapping("/codeinject")
public String codeInject(String filepath) throws IOException {

String[] cmdList = new String[]{"sh", "-c", "ls -la " + filepath};
ProcessBuilder builder = new ProcessBuilder(cmdList);
builder.redirectErrorStream(true);
Process process = builder.start();
return WebUtils.convertStreamToString(process.getInputStream());
}
Comment on lines +24 to +32
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

存在命令注入风险,建议加强参数过滤
codeInject 方法直接将 filepath 拼接到 ls -la 命令中。如果用户传递了恶意命令或分隔符,可能导致任意命令执行。应对输入参数进行严格校验或过滤,或使用更安全的操作方式(如 ProcessBuilder 的可变参数列表而非字符串拼接),示例如下:

- String[] cmdList = new String[]{"sh", "-c", "ls -la " + filepath};
+ // 示例:采用验证后再拼接参数,或将命令和参数分开传递
+ String safePath = SecurityUtil.cmdFilter(filepath);
+ if (safePath == null) {
+     throw new IllegalArgumentException("非法参数");
+ }
+ String[] cmdList = new String[]{"sh", "-c", "ls", "-la", safePath};

Committable suggestion skipped: line range outside the PR's diff.


/**
* Host Injection
* Host: hacked by joychou;cat /etc/passwd
* http://localhost:8080/codeinject/host
*/
@GetMapping("/codeinject/host")
public String codeInjectHost(HttpServletRequest request) throws IOException {

String host = request.getHeader("host");
logger.info(host);
// 使用白名单或其他防护措施,如仅允许合法域名
String safeHost = SecurityUtil.hostFilter(host);
if (safeHost == null) {
throw new IllegalArgumentException("非法或危险的host");
}
String[] cmdList = new String[]{"sh", "-c", "curl " + safeHost};
ProcessBuilder builder = new ProcessBuilder(cmdList);
builder.redirectErrorStream(true);
Process process = builder.start();
return WebUtils.convertStreamToString(process.getInputStream());
}
Comment on lines +34 to +54
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

主机头过滤已实现,但仍可优化命令执行方式

代码已通过SecurityUtil.hostFilter实现了主机头的过滤,增加了安全性。但从最佳实践来看,仍然存在以下问题:

  1. 即使经过过滤,仍然使用shell命令执行curl,这是不必要的风险
  2. 建议使用Java内置的HTTP客户端库替代shell命令执行curl
- String[] cmdList = new String[]{"sh", "-c", "curl " + safeHost};
- ProcessBuilder builder = new ProcessBuilder(cmdList);
- builder.redirectErrorStream(true);
- Process process = builder.start();
- return WebUtils.convertStreamToString(process.getInputStream());
+ // 使用Java的HTTP客户端库
+ URL url = new URL("http://" + safeHost);
+ HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+ return WebUtils.convertStreamToString(conn.getInputStream());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Host Injection
* Host: hacked by joychou;cat /etc/passwd
* http://localhost:8080/codeinject/host
*/
@GetMapping("/codeinject/host")
public String codeInjectHost(HttpServletRequest request) throws IOException {
String host = request.getHeader("host");
logger.info(host);
// 使用白名单或其他防护措施,如仅允许合法域名
String safeHost = SecurityUtil.hostFilter(host);
if (safeHost == null) {
throw new IllegalArgumentException("非法或危险的host");
}
String[] cmdList = new String[]{"sh", "-c", "curl " + safeHost};
ProcessBuilder builder = new ProcessBuilder(cmdList);
builder.redirectErrorStream(true);
Process process = builder.start();
return WebUtils.convertStreamToString(process.getInputStream());
}
/**
* Host Injection
* Host: hacked by joychou;cat /etc/passwd
* http://localhost:8080/codeinject/host
*/
@GetMapping("/codeinject/host")
public String codeInjectHost(HttpServletRequest request) throws IOException {
String host = request.getHeader("host");
logger.info(host);
// 使用白名单或其他防护措施,如仅允许合法域名
String safeHost = SecurityUtil.hostFilter(host);
if (safeHost == null) {
throw new IllegalArgumentException("非法或危险的host");
}
// 使用Java的HTTP客户端库
URL url = new URL("http://" + safeHost);
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
return WebUtils.convertStreamToString(conn.getInputStream());
}


@GetMapping("/codeinject/sec")
public String codeInjectSec(String filepath) throws IOException {
String filterFilePath = SecurityUtil.cmdFilter(filepath);
if (null == filterFilePath) {
return "Bad boy. I got u.";
}
String[] cmdList = new String[]{"sh", "-c", "ls -la " + filterFilePath};
ProcessBuilder builder = new ProcessBuilder(cmdList);
builder.redirectErrorStream(true);
Process process = builder.start();
return WebUtils.convertStreamToString(process.getInputStream());
}
}