114 lines
8.2 KiB
Markdown
114 lines
8.2 KiB
Markdown
# 代码审查报告：性能问题与安全隐患
|
||
|
||
**项目**：bschclassmanage（Spring Boot 2.7.18 + Spring Data JPA + Redis + fastjson 1.2.80 + CAS/云海平台认证）
|
||
**审查范围**：`Repository`、`service/impl/**`、`controller/**`、`utils/**`、`config/**`
|
||
**结论**：发现 **1 个严重、4 个高危、3 个中危、若干低危** 问题。主要集中在 **鉴权越权(IDOR)、敏感信息泄露、Redis 缓存管理接口、TLS 校验关闭、fastjson 反序列化、部分查询缺分页**。
|
||
|
||
---
|
||
|
||
## 🔴 严重 (Critical)
|
||
|
||
### 1. CacheController 提供清空/枚举/删除整个 Redis 的接口，且无角色校验 + 通配 CORS
|
||
- **位置**：`controller/api/CacheController.java:39-166`
|
||
- **接口**：`GET /api/cache/keys`（默认 `pattern=*` 枚举所有 key）、`POST /api/cache/delete`、`POST /api/cache/deletePattern`、`POST /api/cache/clearAll`（`flushDb()`）
|
||
- **风险**：类上 `@CrossOrigin(originPatterns={"*"}, allowCredentials="true")`，且所有方法 **无任何角色/超管校验**。任何已登录用户（含任意学校教师/管理员）都能查看、删除 key，甚至 `flushDb()` 清空整个 Redis 实例，影响 **所有租户** 的缓存与会话；通配 CORS 还可被第三方网页滥用。
|
||
- **修复**：删除 `clearAll`/`deletePattern` 或加超管 `@PreAuthorize`；`keys` 改为 `SCAN`；移除通配 CORS；运维类接口限制内网/独立端口。
|
||
|
||
---
|
||
|
||
## 🟠 高危 (High)
|
||
|
||
### 2. SchCustomGroupService 跨租户越权（IDOR）
|
||
- **位置**：`service/impl/SchCustomGroupService.java:375 / 440 / 515`（`saveMembers` / `setLeaders` / `deleteSchCustomGroup`）
|
||
- **问题**：`teacherCustomGroupRepository.findById(groupId)` 加载组时 **未校验该组属于当前 `user.getSchId()`**，而 `groupId` 直接来自请求 DTO。
|
||
- **风险**：A 校用户可传 B 校的 `groupId`，对 B 校自定义组执行改/删（越权写）。
|
||
- **修复**：改为 `findByIdAndSchId(groupId, user.getSchId())`，或加载后断言 `existing.getSchId().equals(schId)` 再操作。
|
||
|
||
### 3. 字典/课程/高职班级更新接口跨租户越权（6 处）
|
||
- **位置**：
|
||
- `SchDicMajorsService.java:104`
|
||
- `SchDicDivisionService.java:99`
|
||
- `SchDicCampusService.java:104`
|
||
- `SchDicSubjectService.java:187`
|
||
- `SchDicCourseService.java:157`
|
||
- `classes/SchClassHigherService.java:114`
|
||
- **问题**：更新分支均 `findById(id)` 后用 `BeanUtils.copyProperties` 覆盖字段，但 **未校验 `existing.getSchId()==schId`**。同名校验仅限本租户，无法阻止越权改写他校记录的字段（名称/编码等）。
|
||
- **风险**：A 校用户可持 B 校记录 ID 篡改 B 校字典数据。
|
||
- **修复**：用 `findByIdAndSchId` 或在 copy 前加 `existing.getSchId().equals(schId)` 断言。（注意：对应的 `delete` 方法已用 `findByIdAndSchId`，本身是安全的，建议统一。）
|
||
|
||
### 4. HttpUtils 全局禁用 TLS 证书/主机名校验（中间人风险）
|
||
- **位置**：`utils/HttpUtils.java:19-54`（static 块 `trustAllHttpsCertificates()` + `setDefaultHostnameVerifier((h,s)->true)`）
|
||
- **问题**：在类加载时 **对 JVM 内所有 `HttpsURLConnection` 关闭证书链与主机名校验**；`get()` 仅处理 301、无递归深度限制、`contentLen<=0` 时把响应无限读入内存。
|
||
- **风险**：所有出栈 HTTPS 可被中间人劫持/篡改；重定向缺陷可栈溢出或内存耗尽（DoS）。
|
||
- **修复**：仅对特定调用使用显式 `SSLContext`/`TrustManager`（至少校验 CA）；重定向用 `HttpURLConnection` 原生 follow；限制最大响应体积。
|
||
|
||
### 5. 敏感信息暴露：身份证号明文返回
|
||
- **位置**：
|
||
- `SchTeacherService.java:103`（`BeanUtils.copyProperties` 把 `SchTeacher.idCard` 拷入 `SchTeacherVO`，经 `SchTeacherController:44` 返回）
|
||
- `student/SchStudentQueryService.java:326`（拷入 `SchStudentVO`）
|
||
- `TeacherSchVO` / `SchTeacherTOutVO.idCard` 字段
|
||
- **风险**：教师/学生身份证号明文返回前端，违反个人信息保护要求，易泄露。
|
||
- **修复**：VO 去除 `idCard`，或加 `@JsonIgnore`，必要时脱敏（如 `110***********1234`）。
|
||
|
||
---
|
||
|
||
## 🟡 中危 (Medium)
|
||
|
||
### 6. fastjson 反序列化 autoType 风险 + 白名单与包名不匹配
|
||
- **位置**：`config/RedisConfig.java:58-60`（`FastJsonRedisSerializer<Object>` 写入 `@type` + `ParserConfig.addAccept("com.bestsch")`）；`utils/RedisUtil.java:889 / 898 / 921`（`JSON.parseObject/parseArray(JSON.toJSONString(obj), clazz)`）
|
||
- **问题**：fastjson **1.2.80** 存在已知 autoType 绕过 CVE；而 `addAccept("com.bestsch")` 与本项目包名 `com.yunhai.bschclassmanage` **不匹配** —— 白名单既可能失效（领域对象反序列化失败），又未真正起到隔离作用。
|
||
- **修复**：升级 fastjson ≥ 1.2.83（或迁移 fastjson2 并开启 `safeMode`），关闭 autoType；优先改用 Jackson（`GenericJackson2JsonRedisSerializer`）并显式类型；删除 `addAccept("com.bestsch")`。
|
||
- **补充**：`OtherUtil.java:42,59` 的 `JSON.parse` / `JSONObject.parse` 仅用于内部 JSON 校验，未见到 HTTP 直接入口，风险低，但建议统一换成 Jackson。
|
||
|
||
### 7. Redis 阻塞扫描 + 高危死代码（路径遍历）
|
||
- **位置**：
|
||
- `utils/RedisUtil.java:871` `keys(pattern)` 被 `CacheController.java:93,134` 调用（`KEYS` 为 O(N) 阻塞命令，大库会卡死 Redis）
|
||
- `utils/DownloadFileUtil.java:30-36` `new File(filePath)` 路径遍历、`FileInputStream` 未用 try-with-resources 关闭、`System.out.println(file.exists())` 调试残留（当前无调用方，属高危死代码）
|
||
- **修复**：`keys` 改 `SCAN`；`DownloadFileUtil` 限定在基准目录内、try-with-resources、删除调试输出，或直接删除该工具类。
|
||
|
||
### 8. 部分查询缺分页 / 全量加载
|
||
- **位置**：
|
||
- `classes/SchClassQueryService.java:127` `searchBaseSchClass` 返回 `findAll(spec)` 全量
|
||
- `student/SchStudentQueryService.java:202` 学生查询返回 `List` 无 `Pageable`
|
||
- （教师查询已分页，良好）
|
||
- **风险**：大班级/全校学生列表全量返回，内存与响应时间随数据量线性增长。
|
||
- **修复**：改为带 `Pageable` 的分页查询；前端列表必须分页。
|
||
|
||
---
|
||
|
||
## 🟢 低危 / 建议
|
||
|
||
### 9. 资源未关闭
|
||
- `teacher/SchTeacherImportService.java:64` `ExcelImportUtil.importExcel(file.getInputStream(), ...)` 的 `InputStream` 未用 try-with-resources（其余导入服务已用）。
|
||
- **修复**：包进 try-with-resources。
|
||
|
||
### 10. 冗余写
|
||
- `classes/SchClassImportService.java:195/201` 先 `saveAll`，再循环补 `classId` 后二次 `saveAll`。
|
||
- **修复**：先算好 `classId` 再一次性 `saveAll`。
|
||
|
||
### 11. 缓存击穿防护不完整
|
||
- `service/com/ComSchTreeService.getGroupTree` / `getSchoolList` 缓存回填未加锁（`SchClassCacheService` 已用 `synchronized` 双检，正确）。
|
||
- **修复**：对无锁缓存方法加本地/分布式锁，避免并发缓存踩踏。
|
||
|
||
### 12. 纵深防御
|
||
- `SysOperationLogServiceImpl.java:73` `getById(id)` 不带 `schId` 过滤（当前无 Controller 暴露，建议补）。
|
||
- `SchStudent/SchTeacher` 等 `save` 已做 `schId` 校验（良好）。建议统一对所有 `findById` 入口补 `schId` 过滤。
|
||
|
||
---
|
||
|
||
## ✅ 已做好的地方（正面确认）
|
||
- **Repository 的 `@Query` 全部使用命名参数（`:param`）**，无字符串拼接，SQL 注入风险低。
|
||
- **批量更新/升级**（如 `SchClassUpgradeService.batchUpgradeGrade`/`batchGraduate`、学生批量转班）均用单条 `@Modifying` 批量 SQL，**无 N+1**。
|
||
- **四个导入服务均使用 `saveAll` 批量**，无循环单条 `save`。
|
||
- **删除类操作**（`SchClassInfoService`、`SchTeacherInfoService:168`、`SchStudentInfoService:149`、`SchClassTeachInfoService:112`）已做 `schId` 校验。
|
||
|
||
---
|
||
|
||
## 🚑 优先级修复建议
|
||
1. **立即**：`CacheController` 关闭/加超管校验 + 去掉通配 CORS（影响全部租户）
|
||
2. **立即**：`SchCustomGroupService` + 6 个字典更新接口补 `schId` 校验（IDOR）
|
||
3. **立即**：身份证号 VO 脱敏/移除
|
||
4. **尽快**：`HttpUtils` 修复 TLS 校验、重定向、响应体积限制
|
||
5. **尽快**：fastjson 升级/关 autoType，Redis 序列化改 Jackson
|
||
6. **计划**：查询分页、资源关闭、缓存锁、纵深防御
|