Address code review feedback on security fixes

- Fix buffer size for strncpy to match actual buffer (32 bytes)
- Use strtoul instead of strtol for unsigned color values
- Improve strncat bounds checking with accurate length tracking
- Free wordexp results immediately after use instead of batching
- Add strdup for wordexp strings to avoid use-after-free

Co-authored-by: squassina <8495707+squassina@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-02-18 08:57:08 +00:00
parent 9d2f852ec2
commit d017fc4837
4 changed files with 31 additions and 24 deletions

View file

@ -0,0 +1 @@
.

View file

@ -539,7 +539,7 @@ int32_t parse_fold_state(const char *str) {
int64_t parse_color(const char *hex_str) { int64_t parse_color(const char *hex_str) {
char *endptr; char *endptr;
errno = 0; errno = 0;
int64_t hex_num = strtol(hex_str, &endptr, 16); uint64_t hex_num = strtoul(hex_str, &endptr, 16);
// Check for conversion errors // Check for conversion errors
if (*endptr != '\0' || errno == ERANGE) { if (*endptr != '\0' || errno == ERANGE) {
@ -547,11 +547,11 @@ int64_t parse_color(const char *hex_str) {
} }
// Validate range for color values (0x00000000 to 0xFFFFFFFF) // Validate range for color values (0x00000000 to 0xFFFFFFFF)
if (hex_num < 0 || hex_num > 0xFFFFFFFF) { if (hex_num > 0xFFFFFFFF) {
return -1; return -1;
} }
return hex_num; return (int64_t)hex_num;
} }
// 辅助函数:检查字符串是否以指定的前缀开头(忽略大小写) // 辅助函数:检查字符串是否以指定的前缀开头(忽略大小写)
@ -600,17 +600,22 @@ static char *combine_args_until_empty(char *values[], int count) {
combined[0] = '\0'; combined[0] = '\0';
size_t current_len = 0; size_t current_len = 0;
for (int i = 0; i < first_empty; i++) { for (int i = 0; i < first_empty; i++) {
if (i > 0) { if (i > 0 && current_len < total_len) {
size_t remaining = total_len - current_len; size_t remaining = total_len - current_len;
if (remaining > 0) { size_t to_copy = (remaining < 1) ? 0 : 1;
strncat(combined, ",", remaining); if (to_copy > 0) {
current_len += 1; strncat(combined, ",", to_copy);
current_len += to_copy;
} }
} }
size_t remaining = total_len - current_len; if (current_len < total_len) {
if (remaining > 0) { size_t remaining = total_len - current_len;
strncat(combined, values[i], remaining); size_t val_len = strlen(values[i]);
current_len += strlen(values[i]); size_t to_copy = (val_len < remaining) ? val_len : remaining;
if (to_copy > 0) {
strncat(combined, values[i], to_copy);
current_len += to_copy;
}
} }
} }

View file

@ -837,15 +837,17 @@ int32_t spawn(const Arg *arg) {
// 2. 解析参数 // 2. 解析参数
char *argv[64]; char *argv[64];
int32_t argc = 0; int32_t argc = 0;
wordexp_t wordexp_results[63]; // Track all wordexp results for cleanup
int32_t wordexp_count = 0;
char *token = strtok((char *)arg->v, " "); char *token = strtok((char *)arg->v, " ");
while (token != NULL && argc < 63) { while (token != NULL && argc < 63) {
wordexp_t p; wordexp_t p;
if (wordexp(token, &p, 0) == 0) { if (wordexp(token, &p, 0) == 0 && p.we_wordc > 0) {
argv[argc++] = p.we_wordv[0]; // Duplicate the string since we'll free the wordexp result
wordexp_results[wordexp_count++] = p; // Store for cleanup argv[argc] = strdup(p.we_wordv[0]);
wordfree(&p); // Free immediately after copying
if (argv[argc] != NULL) {
argc++;
}
} else { } else {
argv[argc++] = token; argv[argc++] = token;
} }
@ -856,10 +858,9 @@ int32_t spawn(const Arg *arg) {
// 3. 执行命令 // 3. 执行命令
execvp(argv[0], argv); execvp(argv[0], argv);
// 4. execvp 失败时:清理并打印错误 // 4. execvp 失败时:清理分配的字符串并打印错误
for (int i = 0; i < wordexp_count; i++) { // Note: We only need to free strings that were strdup'd from wordexp
wordfree(&wordexp_results[i]); // The original tokens from arg->v don't need to be freed
}
wlr_log(WLR_ERROR, "mango: execvp '%s' failed: %s\n", argv[0], wlr_log(WLR_ERROR, "mango: execvp '%s' failed: %s\n", argv[0],
strerror(errno)); strerror(errno));
_exit(EXIT_FAILURE); // 使用 _exit 避免缓冲区刷新等操作 _exit(EXIT_FAILURE); // 使用 _exit 避免缓冲区刷新等操作

View file

@ -33,8 +33,8 @@ void get_layout_abbr(char *abbr, const char *full_name) {
// 1. 尝试在映射表中查找 // 1. 尝试在映射表中查找
for (int32_t i = 0; layout_mappings[i].full_name != NULL; i++) { for (int32_t i = 0; layout_mappings[i].full_name != NULL; i++) {
if (strcmp(full_name, layout_mappings[i].full_name) == 0) { if (strcmp(full_name, layout_mappings[i].full_name) == 0) {
strncpy(abbr, layout_mappings[i].abbr, 4); strncpy(abbr, layout_mappings[i].abbr, 31);
abbr[4] = '\0'; abbr[31] = '\0';
return; return;
} }
} }
@ -74,8 +74,8 @@ void get_layout_abbr(char *abbr, const char *full_name) {
abbr[2] = '\0'; abbr[2] = '\0';
} else { } else {
// 5. 最终回退:返回 "xx" // 5. 最终回退:返回 "xx"
strncpy(abbr, "xx", 4); strncpy(abbr, "xx", 31);
abbr[4] = '\0'; abbr[31] = '\0';
} }
} }