已关闭
[Bug-Report|缺陷反馈]: 命令注入风险(subprocess shell=True + 参数拼接) #218
P1GGYii创建于  5月7日关闭于  5月25日
P1GGYii
5月7日 创建

Thanks for sending an issue! Please fill in the following template to help quickly solve your problem.

Describe the current behavior / 问题描述 (Mandatory / 必填)

位置:scripts/sign/community_sign_build.py: get_sign_cmd() / _run_sign()

脚本将命令行参数传入的 file 直接通过 str.format() 拼接到 shell 命令字符串中,并使用subprocess.run(..., shell=True)执行。若攻击者可控制传入的文件名/路径(例如包含 ; | && $(...) 等 shell 元字符),将触发命令注入,在构建机上执行任意命令。

cmd = "{} {} {} {}".format(sign_command, file, sign_suffix, sign_crl)
result = subprocess.run(cmd, cwd=mypath, shell=True, check=False, stdout=PIPE, stderr=STDOUT)

Environment / 环境信息 (Mandatory / 必填)

Steps to reproduce the issue / 重现步骤 (Mandatory / 必填)

Describe the expected behavior / 预期结果 (Mandatory / 必填)

此处功能实现无误,出于安全性考虑建议将 subprocess.run(cmd, shell=True) 改为 subprocess.run([..., file, ...], shell=False) 使用参数列表传参,不要用字符串拼接/format() 构造命令,从根源避免命令注入。

Special notes for this issue/备注 (Optional / 选填)

另有其他如下问题,麻烦一并看看

问题1: 命令注入(--sign_script → getstatusoutput())

文件:scripts/sign/add_header_sign.py

CLI 参数 --sign_script 会覆盖 sgn_tool_path,并被直接拼接进命令字符串,最终通过subprocess.getstatusoutput(cmd)执行(该函数固定走 shell)。若传入的 --sign_script 含 shell 元字符(如 ; && | $(...)),可在构建机上执行任意命令。

if hasattr(args, 'sign_script') and args.sign_script:
    sgn_tool_path = args.sign_script

cmd = "{} {} {} {}".format(os.environ["HI_PYTHON"], sign_tool_path, root_dir, file_sign_des)
ret = subprocess.getstatusoutput(cmd)

同样此处建议用 subprocess.run([...], shell=False) 列表传参替代 getstatusoutput(),并对 --sign_script做白名单/目录约束(仅允许仓库内固定脚本路径)。

问题2: 命令注入(XML/CLI 参数拼接 → getstatusoutput())

文件:scripts/sign/add_header_sign.py

脚本将来自 XML 配置的 version/nvcnt/tag/input(以及 CLI 传入的 sign_file_dir 拼出的 input_file)直接插入 f-string 命令字符串,并通过 subprocess.getstatusoutput(cmd)(shell 执行)运行;若 XML 或 CLI 可被不可信方控制,可通过注入 shell 元字符实现任意命令执行。

cmd = f'sudo {os.environ["HI_PYTHON"]} .../esbc_header.py'
cmd += f" -raw_img {input_file} -out_img {input_file}"
cmd += f" -version {conf_item.version} -nvcnt {conf_item.nvcnt} -tag {conf_item.tag}"
ret = subprocess.getstatusoutput(cmd)

同样建议改用 subprocess.run([...], shell=False) 列表传参;并对 XML 属性(version/nvcnt/tag/input)与sign_file_dir做白名单/格式校验与路径约束

问题3: 错误路径内存泄漏(malloc 后 memset_s 失败直接返回)

文件:src/framework/communicator/impl/independent_op/independent_op_context_manager.cc

在创建 Host 侧 context 时,先 malloc(size) 得到 ctxData,随后调用 CHK_SAFETY_FUNC_RET(memset_s(...))。若 memset_s 返回失败,该宏会立即 return,导致已分配的 ctxData 未被 free(),且未写入 contextMap_,无法在 DestroyCommEngineCtx() 中回收,形成错误路径内存泄漏。

ctxData = malloc(size);
CHK_PTR_NULL(ctxData);
CHK_SAFETY_FUNC_RET(memset_s(ctxData, size, 0, size)); // 失败直接返回,ctxData 泄漏

建议把 CHK_SAFETY_FUNC_RET(memset_s(...)) 改成手动判断返回值并在失败时先 free(ctxData)(或用 calloc(1, size) 直接替代 malloc+memset_s)以避免错误路径泄漏。

问题4: 错误路径内存泄漏(malloc 后 memset_s 失败直接返回)

文件:src/framework/next/comms/api_c_adpt/hcomm_c_adpt.cc

在 CPU/CPU_TS/CCU 分支中,先 malloc(size) 赋值给 *ctx,随后调用 CHK_SAFETY_FUNC_RET(memset_s(...))。若 memset_s 失败,该宏会立即 return,导致已分配的 *ctx 未被 free() 且未置空,形成错误路径内存泄漏。

*ctx = malloc(size);
CHK_PTR_NULL(*ctx);
CHK_SAFETY_FUNC_RET(memset_s(*ctx, size, 0, size)); // 失败直接返回,*ctx 泄漏

避免使用会直接返回的宏进行清零;改为检查 memset_s 返回值并在失败时执行free(*ctx); *ctx=nullptr;再返回,或用 calloc(1, size) 替代 malloc + memset_s

问题5:malloc 未判空 + 错误路径内存泄漏(DPU Kernel Init)

文件:src/legacy/framework/communicator/communicator_impl.cc

hostShareBuf = malloc(SHARE_HBM_MEMORY_SIZE);

但未检查返回值是否为 NULL。同时在 malloc 之后存在多处失败直接 return(如获取/切换 ctx、PrepareDpuKernelResource、LaunchDpuKernel 失败等),这些错误路径未释放 hostShareBuf,导致错误路径内存泄漏。

此处加NULL判定成本极低,建议增加校验。

问题6: 空字符串导致 size_t 下溢 → 越界读(OOB Read)

文件:src/platform/hccp/netco/idl/src/bdep/src/v_stringlib.c

VOS_StrToIpAddrCheckValid() 在未检查空字符串的情况下使用:

char scLastChar = *(pscStr + strlen(pscStr) - 1);

pscStr 为 "" 时,strlen(pscStr) - 1 size_t 上下溢为 SIZE_MAX,从而对 pscStr + SIZE_MAX 解引用,触发大范围越界读(未定义行为)。

建议此处在访问 pscStr[len - 1] 前增加空串校验。

likedislike
LLeewis成员
5月7日 关联了看板:HCCL
Leewis成员
5月7日 评论:

感谢反馈,上述所提问题我们需要逐一对脚本/代码进行分析下,待问题确认后尽快反馈修复;

likedislike
LLeewis成员
5月7日 将 rockethcgs 设为负责人
LLeewis成员
5月7日 将 Leewis 设为负责人
pentakill_
5月9日 评论:

问题6: 空字符串导致 size_t 下溢 → 越界读(OOB Read)
文件:src/platform/hccp/netco/idl/src/bdep/src/v_stringlib.c
VOS_StrToIpAddrCheckValid() 在未检查空字符串的情况下使用:
-- 使用这个函数的都在netco仓,属于内部接口,目前内部调用不存在输入空字符串的可能,代码执行不会出现越界读

likedislike
P1GGYii
5月11日 评论:

问题6: 空字符串导致 size_t 下溢 → 越界读(OOB Read)
文件:src/platform/hccp/netco/idl/src/bdep/src/v_stringlib.c
VOS_StrToIpAddrCheckValid() 在未检查空字符串的情况下使用:
-- 使用这个函数的都在netco仓,属于内部接口,目前内部调用不存在输入空字符串的可能,代码执行不会出现越界读

@pentakill_

感谢说明。认可目前的情况:该函数在 netco 仓内作为内部接口使用,现有调用链对入参有约束,确实不存在传入空字符串的场景,因此按当前版本的代码执行路径来看,不会实际触发越界读。

不过从代码健壮性/防御性编程角度,这里对空串缺少保护属于典型边界条件隐患。后续若调用方新增/改动、或解析逻辑出现空字段,风险可能被无意引入。

考虑到在取末字符前增加一次 len==0 判空的改动代价极小、影响面很小、也不改变现有正常输入的行为,建议顺手补上一行空串校验作为加固处理(hardening)。这样可以在不影响现有逻辑的前提下,把潜在 UB 点彻底消除。

likedislike
LLeewis成员
5月11日 将 pentakill_ 设为负责人
pentakill_
5月11日 评论:

问题6: 空字符串导致 size_t 下溢 → 越界读(OOB Read)
文件:src/platform/hccp/netco/idl/src/bdep/src/v_stringlib.c
VOS_StrToIpAddrCheckValid() 在未检查空字符串的情况下使用:
-- 使用这个函数的都在netco仓,属于内部接口,目前内部调用不存在输入空字符串的可能,代码执行不会出现越界读

@pentakill_

感谢说明。认可目前的情况:该函数在 netco 仓内作为内部接口使用,现有调用链对入参有约束,确实不存在传入空字符串的场景,因此按当前版本的代码执行路径来看,不会实际触发越界读。

不过从代码健壮性/防御性编程角度,这里对空串缺少保护属于典型边界条件隐患。后续若调用方新增/改动、或解析逻辑出现空字段,风险可能被无意引入。

考虑到在取末字符前增加一次 len==0 判空的改动代价极小、影响面很小、也不改变现有正常输入的行为,建议顺手补上一行空串校验作为加固处理(hardening)。这样可以在不影响现有逻辑的前提下,把潜在 UB 点彻底消除。

-- 后面加个保护,合入后在这里补充 上对应的PR

likedislike
LLeewis成员
5月19日 关联了pull request:add pscstr check
pentakill_
5月19日 评论:

问题6: 空字符串导致 size_t 下溢 → 越界读(OOB Read)
文件:src/platform/hccp/netco/idl/src/bdep/src/v_stringlib.c
VOS_StrToIpAddrCheckValid() 在未检查空字符串的情况下使用:
-- 使用这个函数的都在netco仓,属于内部接口,目前内部调用不存在输入空字符串的可能,代码执行不会出现越界读

@pentakill_

感谢说明。认可目前的情况:该函数在 netco 仓内作为内部接口使用,现有调用链对入参有约束,确实不存在传入空字符串的场景,因此按当前版本的代码执行路径来看,不会实际触发越界读。

不过从代码健壮性/防御性编程角度,这里对空串缺少保护属于典型边界条件隐患。后续若调用方新增/改动、或解析逻辑出现空字段,风险可能被无意引入。

考虑到在取末字符前增加一次 len==0 判空的改动代价极小、影响面很小、也不改变现有正常输入的行为,建议顺手补上一行空串校验作为加固处理(hardening)。这样可以在不影响现有逻辑的前提下,把潜在 UB 点彻底消除。

-- 后面加个保护,合入后在这里补充 上对应的PR

已闭环:https://gitcode.com/cann/hcomm/pull/2227

likedislike
CANN-robotCANN-robot成员
5月25日 关闭了 issue
CANN-robotCANN-robot成员
5月25日 添加了label:resolved
LLeewis成员
11 天前 移除了看板:HCCL