Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix code injection in extension_utils.py #61180

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ready-research
Copy link

@ready-research ready-research commented Jan 25, 2024

PR types

Bug fixes

PR changes

OPs

Description

This uses shlex for safe command parsing to fix arbitrary code injection. This fixes the arbitrary code execution vulnerability using python stdlib tool shlex, which escapes the characters in the text and makes sure that any parsing does not happen inside a shell. Just like other fix for _wget_download

Like _wget_download() now run_cmd() uses shlex for safe command parsing to fix arbitrary code injection.
@CLAassistant
Copy link

CLAassistant commented Jan 25, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ehtec
Copy link

ehtec commented Jan 25, 2024

People you are doing this wrong, you can't fix this issue like that. There is nothing you can do inside run_cmd function to fix the security problem. The shlex.quote needs to be used on user supplied input in all other functions calling run_cmd. Putting quotes around the whole command will solve nothing, it will just put quotes around the whole command and thus break the application, as you can see in the CI pipeline.

@ready-research
Copy link
Author

@ehtec @wanghuancoder Can you please review this? or suggest a way to fix this issue. Thanks.

I am not sure about check failures.

@paddle-bot paddle-bot bot added the contributor External developers label Jan 25, 2024
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Jan 26, 2024
Copy link
Contributor

@wanghuancoder wanghuancoder left a comment

Choose a reason for hiding this comment

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

目前单测中用到了shell=True这种场景。如果要删掉shell=True。可以对command做一下字符串处理,将command split成多个参数。

@ready-research
Copy link
Author

@wanghuancoder #61285 is fixing this issue?

Copy link

paddle-ci-bot bot commented Mar 15, 2024

Sorry to inform you that 2c308c0's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants