rt.rap: RCE on RAP File Upload

Reported by @melazrak at the Security mailing list

Basic information

Project name: Eclipse RAP

Project id: rt.rap

What are the affected versions?

Not communicated

Details of the issue

I noticed a security issue on org.eclipse.rap.fileupload component and I would like to inform you about it. According to your Security Policy I tried to report vulnerabilities using the Eclipse Foundation's Bugzilla instance but when I created a new account I was asked to have at least one active component in order for me to enter a bug into the product Community. So I am reporting it to you via email.

Remote Code Execution is possible on Windows due to improper filename sanitization for features relying on servicehandler=org.eclipse.rap.fileupload mechanism. A partial sanitization of the filename name is done in the stripFileName method. When this method finds a / it removes everything before but keeps the potential \s. So for the filename "/....\webapps\shell.war" the stripFileName method keeps "....\webapps\shell.war".

Proof when running an app using RAP Fileupload on a Tomcat Server on Windows

The file is saved on webapps folder

Please feel free to ask for more details if needed.

Steps to reproduce

See above

Do you know any mitigations of the issue?

Not communicated

Reported on: August 28, 2023

assigned to @mbarbero, @mrybczyn, @melazrak, @mknauer, and @ifurnadjiev

@mknauer @ifurnadjiev please have a look at the issue reported. Note that you need to be logged in to see it, as it is a confidential issue. Otherwise you will get a 404.

I've tried to add all commiters, but only you two have activated accounts on Eclipse GitLab.

Thank you for reporting it.
We'll check and see what needs to be done.

added From_ML label

The FAQ of the commons-fileupload projects at (https://commons.apache.org/proper/commons-fileupload/faq.html) explains why the full path is present:

and outlines how to retrieve the filename using a utility in commons-io (FilenameUtils#getName). This method gracefully handles the case of separators by using Math.max for any encountered one: https://github.com/apache/commons-io/blob/master/src/main/java/org/apache/commons/io/FilenameUtils.java#L975C20-L975C20

The same should be done in the stripFileName method here imho:

https://github.com/eclipse-rap/org.eclipse.rap/blob/f8823e8a204f22dfca2763411db01ae7f7360068/bundles/org.eclipse.rap.fileupload/src/org/eclipse/rap/fileupload/internal/FileUploadProcessor.java#L135

The fix is waiting a review and will be merged soon.

The created PR seems to be this one: https://github.com/eclipse-rap/org.eclipse.rap/pull/141

I did not reproduce the issue yet, but from the description and inspection of the code, it might be very likely that you can create a remote code exploitation for applications using the RAP fileupload component such that the code will get executed at least once the application restarted if you manage to overwrite application code with uploaded data.

As the PR is already publicly visible, I am not sure how to handle it from there. @mrybczyn

Sure, the pull request is visible.
I don't see any way to avoid that, given that we want to include a fix for the issue as soon as possible, in that case with the next release in about two weeks.

There are now ways to avoid things like that. GitHub supports now security advisories where you can work in a private fork of the repo on a fix and once its ready, release the advisory to the public and merge the fix.

Not sure if we just continue as is, or move that to a security advisory, but we could do that quite easily imho.

This is a relatively new feature, but we have gotten some experience with it already with some other projects in the last couple of weeks.

The commit could have been called 'Fix handling of file names on Windows' for example. This is not a huge protection against someone monitoring the commits for possible exploitation, but gives you time to launch a release.

When we are at this, when do you expect the release? And do you want to assign a CVE to this issue? (I think it is worth it, as we have a potential remote execution)

The pull request #141 Fix handling of file names on Windows and its single commit have been renamed as suggested, the commit is reviewed, passes the tests, and ready to be merged.

If I don't see any objections to this GitLab issue or to the above GitHub pull request by Monday, 2023-09-04 12:00 UTC, we will merge the change into our main branch. From that point on, it will be part of our nightly builds.

It will be released with our already planned Eclipse RAP 3.26 release on 2023-09-13, together with the Eclipse Simultaneous Release 2023-09. Until further notice, no backports to other branches and/or releases are planned.

Regarding CVE: I am not familiar with the current procedures at Eclipse. Where can I find current documentation and help on this topic? Who would ultimately decide and be responsible for creating the CVE? Currently, I am neither reluctant nor really convinced of the need.

Thank you for the explanation @mknauer. We'll be waiting with making this issue public until you release.

Now, for the CVE assignment there's a section in the Handbook here: https://www.eclipse.org/projects/handbook/#vulnerability-cve

In short: either the project or the vulnerability reporter can ask for the CVE to be assigned. What we will need is a description of the issue (this report has enough information), we usually do it in a separate tracker and it is possible to create a new request https://gitlab.eclipse.org/security/cve-assignement/-/issues/new For any field you do not know how to fill, we offer assistance. The whole process has two steps: one is to reserve a number. Then we publish the entry after your fix release. Only at that moment the issue becomes public.

If you have any questions, let us know.

Okay, thank you. We are going to discuss the CVE topic internally, and let you know once we come to a conclusion.

In the above comment I wrote

It will be released with our already planned Eclipse RAP 3.26 release on 2023-09-13, together with the Eclipse Simultaneous Release 2023-09. Until further notice, no backports to other branches and/or releases are planned.

This has changed. We came to the conclusion that it would be nice/friendly/polite not to force everyone to update to the upcoming new release, but we will also provide a hotfix Eclipse RAP 3.25.1 release for our last official release. This release will include only this single change. Everyone else needs either a support contract, or can apply the very same patch that is already in the Git repository.

mentioned in issue cve-assignement#9 (closed)

• The pull request #141 Fix handling of file names on Windows with the fix for this issue has been merged to the main branch of the Eclipse RAP Runtime repository with commit 221bb60. There were no objections raised (here and on the PR itself).
• We, the Eclipse RAP project, ask for assigning a CVE that will become public once we officially publish our next Eclipse RAP 3.26 release scheduled for 2023-09-13. I started the CVE process with cve-assignement#9 (closed). @mrybczyn please have a look and let me know about any missing information.
• Although originally described as "Remote Code Execution", I wonder if this really describes the problem best. For me, the main problem is that FileUpload can change/add files with permissions inherited from the application server. The fact that this can be used for RCE is only one possible side effect. Any thoughts welcome.

I agree that RCE should not be the description of the problem but rather the fact that this vulnerability potentially allows an attacker to overwrite files on the filesystem running the service which could lead in some scenarios to a RCE.

• Can someone with sufficient permissions add @ifurnadjiev to the CVE request issue cve-assignement#9 (closed) and give him the right to read/edit it?
• Can someone with more experience in that area help with the Common Weakness Enumeration and the Common Vulnerability Scoring System? That would be appreciated.

• Can someone with sufficient permissions add @ifurnadjiev to the CVE request issue cve-assignement#9 (closed) and give him the right to read/edit it?

Done.

• Can someone with more experience in that area help with the Common Weakness Enumeration and the Common Vulnerability Scoring System? That would be appreciated.

on it

For CVSS, I'd suggest CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:L (score: 7.3). If you're not used to CVSS, you can check https://www.first.org/cvss/user-guide#Scoring-Rubrics and cvs.js.org

CWE is simpler, and in this case it should be:

• CWE-22: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
• CWE-23: Relative Path Traversal

The new CVE v5 format requires a new field (Common Attack Pattern Enumration and Classification — CAPEC) that is not listed in our issue template. I suggest:

• CAPEC-154: https://capec.mitre.org/data/definitions/154.html

I generally agree to your CVSS scoring suggestion, although I had some deviations in my own initial scoring.

• The attacker needs some kind of server-side session. This was my reason why I came up with a low instead of none value for the required privileges, and with required instead of none for user interaction. But I was led by positive thinking, and yours describes the reality better. I'd suggest to keep yours.
• In my initial judgment, the integrity value was high instead of low. Depending on the application server, there isn't much protection left, and the modification can potentially affect every file that is writable by its process. I'd be leaning towards a high.

Full agreement to your CWE and CAPEC suggestions.

• The attacker needs some kind of server-side session. This was my reason why I came up with a low instead of none value for the required privileges,

I think you're right on this one. Let's use low.

and with required instead of none for user interaction. But I was led by positive thinking, and yours describes the reality better. I'd suggest to keep yours.

In my initial judgment, the integrity value was high instead of low. Depending on the application server, there isn't much protection left, and the modification can potentially affect every file that is writable by its process. I'd be leaning towards a high.

Ok then high it is.

The new CVSS string is now CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:H/A:L, with a score of 7.6

The updated CVSS string looks good to me now. Thank you!

added cveassigned label

@mbarbero We are ready to publish the CVE as mentioned in the other tracking issue at cve-assignement#9 (closed).

I assume you are taking over from here? Please let me know if anything is missing.

The CVE record has been published, so we are closing the ticket and removing the confidentiality flag

closed

made the issue visible to everyone

The CVE entry ticket is cve-assignement#9 (closed)