Skip to content

Conversation

@sxzz
Copy link
Member

@sxzz sxzz commented May 20, 2024

Critical RCE Vulnerability:

An attacker can exploit this vulnerability by uploading files named scripts/size-report.ts, which will overwrite the existing file and subsequently execute the injected code.

This flaw grants the attacker the ability to execute arbitrary code in a secure environment that has write permissions to both pull-requests and issues.

Special thanks to @redyetidev

@sxzz sxzz added security Pull requests that address a security vulnerability 🔥 p5-urgent Priority 5: this fixes build-breaking bugs that affect most users and should be released ASAP. labels May 20, 2024
@github-actions
Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 90.7 kB 34.5 kB 31.1 kB
vue.global.prod.js 148 kB 53.7 kB 48.1 kB

Usages

Name Size Gzip Brotli
createApp 50.8 kB 19.9 kB 18.1 kB
createSSRApp 54.1 kB 21.2 kB 19.3 kB
defineCustomElement 53.1 kB 20.6 kB 18.8 kB
overall 64.5 kB 24.9 kB 22.6 kB

@sxzz sxzz changed the title ci: fix overwrite existing files ci: fix RCE vulnerability in file overwrite May 20, 2024
@sxzz sxzz merged commit 8bf1469 into main May 20, 2024
@sxzz sxzz deleted the ci/fix-overwrite branch May 20, 2024 23:05
@avivkeller
Copy link
Contributor

avivkeller commented May 20, 2024

It was an pleasure to help :-)

sxzz added a commit to vuejs/vue-vapor that referenced this pull request May 21, 2024
@Disservin
Copy link
Contributor

Disservin commented May 21, 2024

out of curiosity, what is wrong with ${{ github.event.number }} ?

@sxzz
Copy link
Member Author

sxzz commented May 21, 2024

size-data and size-report are two distinct workflows. The former runs in an untrusted environment (lacking permissions to write issue/PR comments), while the latter operates in a trusted environment with the necessary permissions.

To link these workflows, we need to pass the target PR number as an argument to the trusted environment via artifacts.

@Disservin
Copy link
Contributor

Sure yes the environments are different, but to me it looks like size-report still knows about the pr environment which triggered it because it still thinks it is a pr

    if: >
      github.event.workflow_run.event == 'pull_request' &&
      github.event.workflow_run.conclusion == 'success'

in my mind replacing

number: ${{ steps.pr-number.outputs.content }}

with

number: ${{ github.event.number }}

should have the same end result without having to upload the pr number in workflow a) and then downloading it in workflow b)

@sxzz
Copy link
Member Author

sxzz commented May 21, 2024

Did you try it? IIRC, it doesn't work. According to GitHub Docs, Webhook event payload is workflow_run object

@Disservin
Copy link
Contributor

I haven't that's why I first asked if i missed anything. I can give it a shot later, the webhook event payload also seems to have an array of objects named pull_requests these objects seem to have the pr number.. i wonder why it is returning an array though

@avivkeller
Copy link
Contributor

avivkeller commented May 21, 2024

As far as I know, pull_requests[0] with the pull_request event will always be the submitted PR.

@sxzz sxzz mentioned this pull request May 28, 2024
lynxlangya pushed a commit to lynxlangya/core that referenced this pull request May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔥 p5-urgent Priority 5: this fixes build-breaking bugs that affect most users and should be released ASAP. security Pull requests that address a security vulnerability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants