Skip to content

fix: use strdup for library name copy in load_libs - #599

Closed
pengfeixx wants to merge 1 commit into
linuxdeepin:masterfrom
pengfeixx:agent/pms-bug-bot/bff69004357f
Closed

pengfeixx wants to merge 1 commit into
linuxdeepin:masterfrom
pengfeixx:agent/pms-bug-bot/bff69004357f

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

fix: use strdup for library name copy in load_libs

  1. Root cause: setLibNames() copied the library path string with an
    unbounded strcpy, flagged by clang-analyzer as CWE-119; the old
    allocation was also leaked on repeat calls and when switching to
    NULL
  2. Fix: free the previous allocation on every call (including the
    switch-to-NULL case), then replace malloc+strcpy with strdup,
    which allocates exactly the source length and copies the
    terminating NUL
  3. Impact: no behavior change for valid inputs; removes the analyzer
    warning and the repeat-call leak

Note: an earlier revision of this PR used
strncpy(dst, src, strlen(src)+1), which is byte-for-byte equivalent
to the original strcpy and provides no safety gain; per review it was
replaced with strdup.

Influence:

  1. Verify input-method candidate libraries still load at editor startup

Log: 使用 strdup 拷贝输入法库名,消除不安全拷贝

@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 @pengfeixx, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 3 days and 15 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pengfeixx

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

@sourcery-ai

sourcery-ai Bot commented Sep 18, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates setLibNames() to use strncpy instead of strcpy when storing the ZPD custom library path, addressing the static-analysis warning without changing expected behavior. Review the allocation/copy size relationship and verify initial setting, replacement, and subsequent library loading.

Sequence diagram for setting and loading the ZPD library path

sequenceDiagram
    participant Caller
    participant load_libs
    participant Heap
    participant ZPDLibrary

    Caller->>load_libs: setLibNames(tmp)
    alt existing path
        load_libs->>Heap: free(g_ldnames.chZPDDLL)
    end
    load_libs->>Heap: malloc(strlen(tmp.chZPDDLL)+1)
    load_libs->>load_libs: strncpy(g_ldnames.chZPDDLL, tmp.chZPDDLL, strlen(tmp.chZPDDLL)+1)
    Caller->>ZPDLibrary: load using g_ldnames.chZPDDLL
Loading

File-Level Changes

Change Details Files
Replaced the unbounded library-path copy with a length-limited copy while preserving the existing allocation and behavior.
  • Keep allocating storage for the source path plus its null terminator.
  • Use strncpy with the allocation-sized length argument.
  • Retain existing handling for replacing a previously configured path.
src/basepub/load_libs.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 99 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 总体评分 99 分,大于 70 分通过阈值,代码质量符合要求。本次变更将不安全的 strcpy 替换为 strncpy,消除了 CWE-119 静态分析告警,行为完全等价,无功能变化。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 语法正确,逻辑清晰。strncpy 调用语法正确,参数 strlen(tmp.chZPDDLL)+1 与 malloc 分配大小一致,行为与原 strcpy 完全等价。NULL 检查逻辑完善(line 80-81 处理 NULL 输入,line 84-85 释放旧内存)。


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 代码结构清晰,注释完整。函数简短聚焦,职责单一。line 83 有注释说明"重复调用时先释放旧的分配,避免内存泄漏"。无重复代码,无调试残留。


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. src/basepub/load_libs.c:88 - strlen(tmp.chZPDDLL) 在 line 87 和 line 88 被调用了两次,可提取为局部变量避免重复计算

建议: 性能良好,资源使用合理。轻微建议:将 strlen(tmp.chZPDDLL) 的结果存储到局部变量中,避免重复计算。建议优化:size_t len = strlen(tmp.chZPDDLL) + 1; g_ldnames.chZPDDLL = (char*)malloc(len); strncpy(g_ldnames.chZPDDLL, tmp.chZPDDLL, len);


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: 存在0个安全漏洞。本次变更将不安全的 strcpy 替换为 strncpy,消除了 clang-analyzer 标记的 CWE-119 不安全 API 告警。缓冲区大小与复制长度一致,不存在缓冲区溢出风险。无硬编码密钥、无注入风险。


💡 改进建议代码示例

// 优化建议:提取 strlen 结果为局部变量,避免重复计算
void setLibNames(LoadLibNames tmp)
{
    if(tmp.chZPDDLL == NULL) {
        g_ldnames.chZPDDLL = NULL;
    } else {
        // 重复调用时先释放旧的分配,避免内存泄漏
        if (g_ldnames.chZPDDLL != NULL) {
            free(g_ldnames.chZPDDLL);
        }
        size_t len = strlen(tmp.chZPDDLL) + 1;
        g_ldnames.chZPDDLL = (char*)malloc(len);
        strncpy(g_ldnames.chZPDDLL, tmp.chZPDDLL, len);
    }
}

本报告由 AI 代码审查工具自动生成

1. Root cause: setLibNames copied the library name with strcpy into a
   freshly malloc'd buffer, flagged by clang-analyzer as an unbounded
   copy (CWE-119); the previous allocation was also leaked on repeat
   calls and when switching to NULL
2. Fix: free the previous allocation on every call (including the
   switch-to-NULL case) and replace malloc+strcpy with strdup, which
   allocates exactly the source length and copies the terminating NUL
3. Impact: no behavior change for valid inputs; removes the analyzer
   warning and the repeat-call leak

Influence:
1. Verify input-method candidate libraries still load at editor startup

fix: 使用 strdup 拷贝输入法库名,消除不安全拷贝

1. 根因:setLibNames 使用 strcpy 向新分配缓冲拷贝库名,被
   clang-analyzer 标记为无界拷贝(CWE-119);重复调用及切换为
   NULL 时旧分配泄漏
2. 方案:每次调用(含切换为 NULL)先释放旧分配,并用 strdup
   替代 malloc+strcpy,按源串精确长度分配并拷贝(含结尾 '\0')
3. 影响:有效输入下行为不变;消除分析器告警与重复调用泄漏

Influence:
1. 验证编辑器启动时输入法候选库正常加载
@pengfeixx pengfeixx closed this Sep 20, 2026
@pengfeixx
pengfeixx force-pushed the agent/pms-bug-bot/bff69004357f branch from 96c279a to 3f1e0ed Compare September 20, 2026 02:14
@pengfeixx pengfeixx changed the title fix: replace insecure strcpy with strncpy in load_libs fix: use strdup for library name copy in load_libs Sep 20, 2026
@pengfeixx

Copy link
Copy Markdown
Contributor Author

Superseded by #603 (same branch): per review, strncpy(strlen+1) was replaced with strdup, and the old allocation is freed on every call including switch-to-NULL. This PR's diff view was corrupted by a force-push that replaced a shallow-clone root commit.

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.

2 participants