Skip to content

refactor: optimize power key screen off logic and add safe file ops#1149

Merged
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:master
Jun 17, 2026
Merged

refactor: optimize power key screen off logic and add safe file ops#1149
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602

@fly602 fly602 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor
  1. Add SafeReadFile and SafeWriteFile utilities to prevent symlink attacks
  2. Check DPMS state before turning off screen to avoid redundant operations
  3. Change file permissions from 0644 to 0600 for security hardening
  4. Delegate power key screen toggle to x-event idle off mechanism

Influence:

  1. Test power key screen off behavior when screen is already off
  2. Verify DPMS state file is correctly read and written
  3. Test screen off function does not execute when already in DPMS off state
  4. Verify file operations reject symlink-based attacks
  5. Test power key behavior with various screen states
  6. Verify /tmp/dpms-state file permissions are secure

fix: 优化电源键关闭显示器逻辑并加固文件操作安全

  1. 新增 SafeReadFile 和 SafeWriteFile 工具函数防止符号链接攻击
  2. 关闭屏幕前检查 DPMS 状态以避免重复操作
  3. 将文件权限从 0644 改为 0600 以加强安全性
  4. 将电源键亮屏功能交由 x-event idle off 机制处理

Influence:

  1. 测试屏幕已关闭时电源键的行为
  2. 验证 DPMS 状态文件的正确读写
  3. 测试 DPMS 已关闭状态下不会重复执行关屏操作
  4. 验证文件操作能正确拒绝符号链接攻击
  5. 测试不同屏幕状态下的电源键行为
  6. 验证 /tmp/dpms-state 文件权限的安全性

PMS: BUG-364835

1. Add SafeReadFile and SafeWriteFile utilities to prevent symlink
attacks
2. Check DPMS state before turning off screen to avoid redundant
operations
3. Change file permissions from 0644 to 0600 for security hardening
4. Delegate power key screen toggle to x-event idle off mechanism

Influence:
1. Test power key screen off behavior when screen is already off
2. Verify DPMS state file is correctly read and written
3. Test screen off function does not execute when already in DPMS off
state
4. Verify file operations reject symlink-based attacks
5. Test power key behavior with various screen states
6. Verify /tmp/dpms-state file permissions are secure

fix: 优化电源键关闭显示器逻辑并加固文件操作安全

1. 新增 SafeReadFile 和 SafeWriteFile 工具函数防止符号链接攻击
2. 关闭屏幕前检查 DPMS 状态以避免重复操作
3. 将文件权限从 0644 改为 0600 以加强安全性
4. 将电源键亮屏功能交由 x-event idle off 机制处理

Influence:
1. 测试屏幕已关闭时电源键的行为
2. 验证 DPMS 状态文件的正确读写
3. 测试 DPMS 已关闭状态下不会重复执行关屏操作
4. 验证文件操作能正确拒绝符号链接攻击
5. 测试不同屏幕状态下的电源键行为
6. 验证 /tmp/dpms-state 文件权限的安全性

PMS: BUG-364835

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @fly602, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:85分

■ 【总体评价】

代码正确修复了TOCTOU符号链接攻击漏洞并收紧了文件权限,但存在硬编码路径和状态判断逻辑不一致等质量问题
逻辑正确但因硬编码及错误处理缺失扣15分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

SafeReadFile与SafeWriteFile通过Lstat与O_NOFOLLOW结合,有效消除了竞态条件,整体逻辑严密无编译错误
建议:无需修改

  • 2.代码质量(一般)✕

/tmp/dpms-state路径在三个不同文件中硬编码,违反DRY原则;restoreDpmsStateFile中直接使用string(v) == "1"判断,而isDpmsOff中使用了bytes.TrimSpace,两者逻辑不对称;systemTurnOffScreen中忽略了SafeWriteFile的返回错误
潜在问题:状态判断不一致可能导致恢复逻辑失效;忽略写入错误可能导致异常状态未被记录
建议:将路径提取为fileutil包内的公共常量;统一使用bytes.TrimSpace处理读取内容;补充SafeWriteFile的错误日志记录

  • 3.代码性能(无性能问题)✓

增加的Lstat系统调用对于关闭屏幕等低频操作而言开销可忽略不计,无性能瓶颈
建议:无需修改

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码通过O_NOFOLLOW内核级拒绝与O_CREAT|O_EXCL原子创建彻底阻断了符号链接替换攻击,权限从0644收紧至0600防止了信息泄露,安全防护机制完善
建议:无需修改

■ 【改进建议代码示例】

// common/fileutil/fileutil.go 顶部新增常量定义
const DpmsStateFile = "/tmp/dpms-state"

// keybinding1/utils.go 修改 systemTurnOffScreen 和 isDpmsOff
func (m *Manager) systemTurnOffScreen() {
	if isDpmsOff() {
		return
	}
	logger.Info("DPMS Off")
	// ... (省略中间 DPMS 关闭逻辑) ...
	undoPrepareSuspend()
	if err := fileutil.SafeWriteFile(fileutil.DpmsStateFile, []byte("1"), 0600); err != nil {
		logger.Warning("write dpms state failed:", err)
	}
}

func isDpmsOff() bool {
	content, err := fileutil.SafeReadFile(fileutil.DpmsStateFile)
	if err != nil {
		logger.Debug("read dpms state error:", err)
		return false
	}
	return bytes.Equal(bytes.TrimSpace(content), []byte("1"))
}

// session/power1/power_save_plan.go 修改 restoreDpmsStateFile
func (psp *powerSavePlan) restoreDpmsStateFile() {
	v, err := fileutil.SafeReadFile(fileutil.DpmsStateFile)
	if err != nil {
		return
	}

	if bytes.Equal(bytes.TrimSpace(v), []byte("1")) {
		err = fileutil.SafeWriteFile(fileutil.DpmsStateFile, []byte("0"), 0600)
		if err != nil {
			logger.Warning("write dpms state:", err)
		}
	}
}

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fly602 fly602 merged commit d970a19 into linuxdeepin:master Jun 17, 2026
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants