Fix Path Traversal fallback vulnerability

[MorielHarush]

Vulnerability Summary
The library fails to validate the resulting path of the fallback mechanism against the configured root directories. While the primary file lookup paths are protected by the contains() check, the fallback path yields the file path directly without verifying if it stays within the restricted boundaries.

Proof of Concept (PoC)

const { Liquid } = require('liquidjs');
const e = new Liquid({ root: ['/tmp'], partials: ['/tmp'], dynamicPartials: true });
e.parseAndRender('{% include page %}', { page: '../../../etc/passwd' }).then(o => console.log(o.slice(0,500)));

Technical Analysis
The issue resides in src/fs/loader.ts within the candidates() generator. Lines 49 and 58 correctly apply the enforceRoot and this.contains(dir, filepath) checks. However, the fallback block (Lines 62-65) lacks this security gate.

Vulnerable Code (loader.ts):

if (fs.fallback !== undefined) {
  const filepath = fs.fallback(file)
  if (filepath !== undefined) yield filepath
}

Suggested Fix
The fallback result should only be yielded if it satisfies the same security contract as the other candidate paths. I suggest the following modification:

if (fs.fallback !== undefined) {
  const filepath = fs.fallback(file)
  if (filepath !== undefined) {
    if (!enforceRoot || dirs.some(dir => this.contains(dir, filepath))) {
      yield filepath
    }
  }
}

This ensures that the fallback path is only allowed if it falls within one of the configured root directories, matching the intended security behavior of the library.

Impact
This is a high-severity issue for any application that passes user-controlled input to tags like {% include %} or {% render %} while relying on the root setting for isolation.

[coveralls]

Pull Request Test Coverage Report for Build 22091788376

Details

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.03%) to 99.85%

---

Totals
Change from base Build 22012273213:	-0.03%
Covered Lines:	2908
Relevant Lines:	2910

---

💛 - Coveralls

[MorielHarush]

@harttle
Hey , any concerns ?

[harttle]

can we have a integration test case which demonstrates the issue?

[MorielHarush]

This is the test case :
"
const { Liquid } = require('liquidjs');
const e = new Liquid({ root: ['/tmp'], partials: ['/tmp'], dynamicPartials: true });
e.parseAndRender('{% include page %}', { page: '../../../etc/passwd' }).then(o => console.log(o.slice(0,500)));
"

[harttle]

@harttle Hey , any concerns ?

sorry, commented.

[MorielHarush]

@harttle Bumping :)

[peaktwilight]

I independently discovered the same root cause through a different attack vector: unquoted absolute paths with dynamicPartials: false (e.g. {% render /etc/passwd %}). The fallback block in *candidates yields without contains() > your fix here covers both vectors. I've confirmed this is exploitable on 10.24.0.

I've also emailed the @harttle per SECURITY.md since this has security implications for downstream dependents.
I won't namedrop any projects here, but it affects some really well-known open-source projects that use LiquidJS.

Would be great to get this merged and released asap 😄

[MorielHarush]

I independently discovered the same root cause through a different attack vector: unquoted absolute paths with dynamicPartials: false (e.g. {% render /etc/passwd %}). The fallback block in *candidates yields without contains() > your fix here covers both vectors. I've confirmed this is exploitable on 10.24.0.

I've also emailed the @harttle per SECURITY.md since this has security implications for downstream dependents. I won't namedrop any projects here, but it affects some really well-known open-source projects that use LiquidJS.

Would be great to get this merged and released asap 😄

Hey can you email me the open-source projects which is vulnerable ? for the record.
harushmoriel@gmail.com.

@harttle Hey Buddy, this needs to be CVE signed as well.. when you prepare to merge the fix ?

[harttle]

Fixed pipeline issues and merged in #855

[peaktwilight]

@harttle thanks for the including the fix and the new hotfix release.

It would obviously be great to create a CVE and or advisory to urge dependant software to upgrade asap with the reasoning and full bug analysis. I'd be very happy to get credited together with @MorielHarush with our respective PoCs.

For the record, now that there's an update with a fix available, I assume that it's fair to release my version of the PoC publicly for the sake of transparency and documentation. Let me know if you agree, and I'll soon add a writeup about the bug and link it here.

@MorielHarush currently in contact with another dev from another big open-source project regarding their implementation and how this bug affects their security. Will include it in the writeup once they upgrade their liquidjs version.

[harttle]

@peaktwilight OK for me. Thanks for driving this.

[peaktwilight]

Awesome, thanks again @harttle for reacting so quickly!!

I think I'll wait on the writeup till we have an official CVE, since the repo owner of the other project that I know is affected is currently working on a release with the version bump. I don't want to publicize the PoC before it's fixed downstream there as it's a repo with 50k+ stars so lots of users would potentially be under threat...

Also looks like @MorielHarush or I cannot initiate the GitHub Security Advisory to request the CVE for this.

Here's how you can initiate it though:

1. In your repo > Security tab > Advisories > New draft security advisory
2. Details like:
   - Ecosystem: npm
   - Package: liquidjs
   - Affected versions: < 10.25.0
   - Patched version: 10.25.0
   - Severity: High
   - CWE: CWE-22 (Improper Limitation of a Pathname to a Restricted Directory)
   - Description: The fallback block in the *candidates() generator in src/fs/loader.ts yields file paths without applying the enforceRoot / contains() check, allowing arbitrary file reads via path traversal when user-controlled input reaches {% include %} or {% render %} tags. You can gladly link the private Gist that I sent you over email for my PoC together with @MorielHarush 's PoC
   - Credit: Moriel and I as reporters
3. Check "Request CVE ID" at the bottom > GitHub is a CNA and should assign one automatically
4. Publish the advisory

The main advantage of doing this is that once the advisory / CVE exists, Dependabot and other dependency scanners will automatically flag every downstream project still on a vulnerable version. Without it, those projects have no way of knowing they need to upgrade. Given how widely your awesome project is used, that's a lot of exposed applications.

I'll reference the CVE that gets created here in my writeup to help raise awareness to those who might be affected.
Thanks so much for helping us getting through this process!

[MorielHarush]

hey @peaktwilight and @harttle ,
The vulnerability was already fully documented and reported in my initial submission. According to standard CVE/CNP guidelines, credit is assigned to the original reporter. While your additional PoC is noted, the core discovery and reproduction steps were already established prior to your comment.
This means that the credit for this CVE belongs to me alone.

[harttle]

I've also emailed the @harttle per SECURITY.md since this has security implications for downstream dependents.

Hi @MorielHarush, I received an email with repro steps for the same issue on 30 Dec, 2025 from caplanmaor@gmail.com If this guy was you @peaktwilight , could you please email me again confirming your Github username?

[peaktwilight]

Hey @MorielHarush, no disagreement that you found and reported the bug first > no one's disputing that.

I don't know where you have that info from but CVE credit guidelines from MITRE's and GitHub's own CNA process routinely credit multiple independent reporters, even more so when they discovered different attack vectors.

For the record, the CVE 5.0 schema defines credits as an array specifically designed for multiple entries (https://cvelab.github.io/cve-schema/schema/v5.0/docs/). GitHub's own advisory system supports the same multiple credit types: finder, reporter, analyst, coordinator, etc. (https://github.blog/changelog/2023-03-07-security-advisories-now-have-multiple-types-of-credits/, https://docs.github.com/en/code-security/security-advisories/working-with-repository-security-advisories/creating-a-repository-security-advisory). There is no "sole credit" rule in any CVE or CNA guideline.

Our discoveries are also distinct:

• Your vector: relative path traversal with dynamicPartials: true (../../../etc/passwd)
• My vector: unquoted absolute paths with dynamicPartials: false ({% render /etc/passwd %})

These are different exploitation paths that happen to share the same root cause in the fallback block.

I'm not trying to take credit from your hard work. I also independently emailed @harttle per SECURITY.md, provided a separate PoC with a seperate attack vector and writeup, identified and contacted affected downstream maintainers first, and helped drive the advisory/CVE process here. It also just so happens that I randomly found out about this issue thanks to the more popular repo downstream and we're fixing it there atm. Funnily enough, I wasn't even searching for vulnerabilities specifically in liquidjs at the time.

I will let @harttle do what he thinks is fair in this situation but I think it's pretty obivious with these hard facts and the whole context. I'm ok to be listed as a secondary reporter if ordering matters to you, but removing credit entirely for two seperate issues wouldn't be accurate.

Either way, nice catch on the original find and glad we both spotted it.

On top of this @harttle, no email does not belong to me, looks like we might have someone who spotted something even earlier? Can you describe his PoC attack vector please @harttle ? In any case, we've finally fixed the issue which is great, the main show shouldn't be about who got the credit :)

[MorielHarush]

I've also emailed the @harttle per SECURITY.md since this has security implications for downstream dependents.

Hi @MorielHarush, I received an email with repro steps for the same issue on 30 Dec, 2025 from caplanmaor@gmail.com If this guy was you @peaktwilight , could you please email me again confirming your Github username?

Hey,
Yeah maor caplan is my team mate.
You should credit him as well.
Thats not peaktwilight.

[MorielHarush]

Hey @MorielHarush, no disagreement that you found and reported the bug first > no one's disputing that.

I don't know where you have that info from but CVE credit guidelines from MITRE's and GitHub's own CNA process routinely credit multiple independent reporters, even more so when they discovered different attack vectors.

For the record, the CVE 5.0 schema defines credits as an array specifically designed for multiple entries (https://cvelab.github.io/cve-schema/schema/v5.0/docs/). GitHub's own advisory system supports the same multiple credit types: finder, reporter, analyst, coordinator, etc. (https://github.blog/changelog/2023-03-07-security-advisories-now-have-multiple-types-of-credits/, https://docs.github.com/en/code-security/security-advisories/working-with-repository-security-advisories/creating-a-repository-security-advisory). There is no "sole credit" rule in any CVE or CNA guideline.

Our discoveries are also distinct:

• Your vector: relative path traversal with dynamicPartials: true (../../../etc/passwd)

• My vector: unquoted absolute paths with dynamicPartials: false ({% render /etc/passwd %})

These are different exploitation paths that happen to share the same root cause in the fallback block.

I'm not trying to take credit from your hard work. I also independently emailed @harttle per SECURITY.md, provided a separate PoC with a seperate attack vector and writeup, identified and contacted affected downstream maintainers first, and helped drive the advisory/CVE process here. It also just so happens that I randomly found out about this issue thanks to the more popular repo downstream and we're fixing it there atm. Funnily enough, I wasn't even searching for vulnerabilities specifically in liquidjs at the time.

I will let @harttle do what he thinks is fair in this situation but I think it's pretty obivious with these hard facts and the whole context. I'm ok to be listed as a secondary reporter if ordering matters to you, but removing credit entirely for two seperate issues wouldn't be accurate.

Either way, nice catch on the original find and glad we both spotted it.

On top of this @harttle, no email does not belong to me, looks like we might have someone who spotted something even earlier? Can you describe his PoC attack vector please @harttle ? In any case, we've finally fixed the issue which is great, the main show shouldn't be about who got the credit :)

I have above 80 cves..
i know how that system works.
Im sorry but you shouldn't be credited .

[peaktwilight]

Not worth arguing if this is where it's going hahah

I've cited the actual CVE 5.0 schema docs, GitHub's CNA documentation, and their credit type system > none of which support any kind of "sole credit" rule, especially for two separate attack vectors. Even for the same vector, CVEs routinely attribute multiple independent reporters. "I know how that system works" isn't a counter-argument to official documentation.

@harttle I'll leave the credit decision to you. Depending on your choice, I'll reference this in a writeup after it's also fixed in the affected downstream project I'm coordinating with. That advisory isn't live yet as we are waiting on a new release over there first.

@harttle Thanks again for the quick fix and release, as I said, that's what matters most here.

[MorielHarush]

Hey @peaktwilight,

The PoC you shared uses the exact same attack vector and targets the same fallback code already identified in my original report.

While I appreciate the interest, credit for a CVE is awarded to the original discoverer. Since Maor and I spent months researching this and provided the initial disclosure and PoC, the credit belongs solely to us.

Furthermore, please refrain from publishing a writeup for a vulnerability you did not discover at first. It is standard industry ethics to leave the documentation and writeups to the researchers who found and reported the flaw. Let’s keep the credit where it’s due.

I'm not going to reply anymore about that.

@harttle Im waiting for maor caplan to wake up he is in the US for a vacation for replying here about it as well.
adding maor @maorcap.

Credit belongs to -
@MorielHarush @maorcap

[harttle]

Hi guys, there're multiple reports for this vulnerability and most of them are via email. I would like to make it transparent to honor all contributors.

Timeline (TZ +0800)

1. @maorcap 30th Dec 2025 via email, repro: include and render tags with dynamicPartials: true
2. @MorielHarush 17th Feb 2026 via email and also this PR, repro: include tag with dynamicPartials: true
3. @ByamB4 22th Feb 2026 via email, repro: include, render, layout tags with string literal path or variable when dynamicPartials: true
4. @peaktwilight 7th March 2026 via email, repro: render tag with literal path and dynamicPartials: false

Attack Vectors

• As dynamicPartials defaults to true, both {% include "/etc/passwd" %} and {% include pswdPath %} works.
• All of render, layout and include shares same lookup, thus all these tags are vulnerable.

Credits

• Finder: @maorcap filed the initial report.
• Remediation developer: @MorielHarush filed this MR changing correct part. Not independent from @maorcap thus not a Finder, but I can change you to a Finder/Analyst per @maorcap's request.
• Analyst: @ByamB4 expanded the scope to all 3 tags, and also a test case for literal string path.
• @peaktwilight Thanks for your effort but according to the timeline there's no new information on your report thus no credit on the CVE according to First Reporter Principle.

Please do help review the content of this advisory: GHSA-wmfp-5q7x-987x Thanks all for the collaboration.

[peaktwilight]

@harttle
Thanks for creating the advisory and the transparent timeline!

The description looks good to me, might be worth mentioning the require.resolve() fallback as the specific root cause for documentation purposes.

Also, adding that dynamicPartials: false was not an effective mitigation > the downstream project that I was talking about earlier relied on it as a security boundary. Worth flagging so others don't assume that setting protects them on pre-10.25.0 versions.

Regardless, I totally respect your decision if you think all the above isn't worth crediting for.
Wishing a nice day to both of you