-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Remote Code Execution vulnerability in Dynamic JSON/TOML/YAML badges #10553
Labels
security
Refer to our SECURITY.md policy before opening pull requests that address a security vulnerability
Comments
chris48s
added
the
security
Refer to our SECURITY.md policy before opening pull requests that address a security vulnerability
label
Sep 25, 2024
TheCaptain989
added a commit
to TheCaptain989/docker-mods
that referenced
this issue
Sep 26, 2024
TheCaptain989
added a commit
to TheCaptain989/docker-mods
that referenced
this issue
Sep 26, 2024
Closed
2 tasks
I tried to make a json to xml proxy at |
4 tasks
think this has done the job as a pinned issue now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
security
Refer to our SECURITY.md policy before opening pull requests that address a security vulnerability
We've just published a critical security advisory relating to a Remote Code Execution vulnerability in Dynamic JSON/TOML/YAML badges: GHSA-rxvx-x284-4445 Thanks to @nickcopi for his help with this.
If you self-host your own instance of Shields you should upgrade to server-2024-09-25 or later as soon as possible to protect your instance.
This is primarily a concern for self-hosting users. However this does also have a couple of knock-on implications for some users of shields.io itself.
1. Users who have authorized the Shields.io GitHub OAuth app
While we have taken steps to close this vulnerability quickly after becoming aware of it, this attack vector has existed in our application for some time. We aren't aware of it having been actively exploited on shields.io. We also can't prove that it has not been exploited.
We don't log or track our users, so a breach offers a very limited attack surface against end users of shields.io. This is by design. One of the (few) information assets shields.io does hold is our GitHub token pool. This allows users to share a token with us by authorizing our OAuth app. Doing this gives us access to a token with read-only access to public data which we can use to increase our rate limit when making calls to the GitHub API.
The tokens we hold are not of high value to an attacker because they have read-only access to public data, but we can't say for sure they haven't been exfiltrated. If you've donated a token in the past and want to revoke it, you can revoke the Shields.io OAuth app at https://github.com/settings/applications which will de-activate the token you have shared with us.
2. Users of Dynamic JSON/TOML/YAML badges
Up until now, we have been using https://github.com/dchester/jsonpath as our library querying documents using JSONPath expressions. @nickcopi responsibly reported to us how a prototype pollution vulnerability in this library could be exploited to construct a JSONPath expression allowing an attacker to perform remote code execution. This vulnerability was reported on the package's issue tracker but not flagged by security scanning tools. It seems extremely unlikely that this will be fixed in the upstream package despite being widely used. It also seems unlikely this package will receive any further maintenance in future, even in response to critical security issues. In order to resolve this issue, we needed to switch to a different JSONPath library. We've decided to switch https://github.com/JSONPath-Plus/JSONPath using the
eval: false
option to disable script expressions.This is an important security improvement and we have to make a change to protect the security of shields.io and users hosting their own instance of the application. However, this does come with some tradeoffs from a backwards-compatibility perspective.
Using
eval: false
Using JSONPath-Plus with
eval: false
does disable some query syntax which relies on evaluating javascript expressions.For example, it would previously have been possible to use a JSONPath query like
$..keywords[(@.length-1)]
against the document https://github.com/badges/shields/raw/master/package.json to select the last element from the keywords arrayshields/package.json
Lines 6 to 12 in e237e40
This is now not a supported query.
In this particular case, you could rewrite that query to
$..keywords[-1:]
and obtain the same result, but that may not be possible in all cases. We do realise that this removes some functionality that previously worked but closing this remote code execution vulnerability is the top priority, especially since there will be workarounds in many cases.Implementation Quirks
Historically, every JSONPath implementation has had its own idiosyncrasies. While most simple and common queries will behave the same way across different implementations, switching to another library will mean that some subset of queries may not work or produce different results.
One interesting thing that has happened in the world of JSONPath lately is RFC 9535 https://www.rfc-editor.org/rfc/rfc9535 which is an attempt to standardise JSONPath. As part of this mitigation, we did look at whether it would be possible to migrate to something that is RFC9535-compliant. However it is our assessment that the JavaScript community does not yet have a sufficiently mature/supported RFC9535-compliant JSONPath implementation. This means we are switching from one quirky implementation to another implementation with different quirks.
Again, this represents an unfortunate break in backwards-compatibility. However, it was necessary to prioritise closing off this remote code execution vulnerability over backwards-compatibility.
Although we can not provide a precise migration guide, here is a table of query types where https://github.com/dchester/jsonpath and https://github.com/JSONPath-Plus/JSONPath are known to diverge from the consensus implementation. This is sourced from the excellent https://cburgmer.github.io/json-path-comparison/
While this is a long list, many of these inputs represent edge cases or pathological inputs rather than common usage.
Table
$[2:-113667776004:-1]
$[113667776004:2:-1]
$[3:0:-2]
$[7:3:-1]
$[::-2]
$[3::-1]
$[:2:-1]
$[0:0]
$[0:3:0]
$[010:024:010]
$[]
$[0]
$[0]
$[-1]
$[':']
$[']']
$['@']
$['\\']
$['\'']
$['$']
$[':@."$,*\'\\']
$['single'quote']
$[',']
$['*']
$['*']
$[ 'a' ]
$['two'.'some']
$[two.some]
$[key]
@.a
$.['key']
$.["key"]
$.[key]
$...key
$['one','three'].key
$.key-dash
$."key"
$.."key"
$.
$.length
$.$
$.??
$.2
$.-1
$.'key'
$..'key'
$.'some.key'
$. a
$..*
$a
.key
key
n/a
$[?(@.key)]
$..*[?(@.id>2)]
$..[?(@.id==2)]
$[?(@.key+50==100)]
$[?(@.key>0 && false)]
$[?(@.key>0 && true)]
$[?(@.key>0 || false)]
$[?(@.key>0 || true)]
$[?(@[-1]==2)]
$[?(@[1]=='b')]
$[?(@)]
$[?(@.a && @.b || @.c)]
$[?(@.key/10==5)]
$[?(@.key-dash == 'value')]
$[?(@.2 == 'second')]
$[?(@.2 == 'third')]
$[?()]
$[?(@.key==42)]
$[?(@==42)]
$[?(@.key==42)]
$[?(@.d==["v1","v2"])]
$[?(@[0:1]==[1])]
$[?(@.*==[1,2])]
$[?(@.d==["v1","v2"] || (@.d == true))]
$[?(@.d==['v1','v2'])]
$[?((@.key<44)==false)]
$[?(@.key==false)]
$[?(@.key==null)]
$[?(@[0:1]==1)]
$[?(@[*]==2)]
$[?(@.*==2)]
$[?(@.key==-0.123e2)]
$[?(@.key==010)]
$[?(@.d=={"k":"v"})]
$[?(@.key=="value")]
$[?(@.key=="Mot\u00f6rhead")]
$[?(@.key==true)]
$[?(@[email protected])]
$.items[?(@.key==$.value)]
$[?(@.key>42)]
$[?(@.key>=42)]
$[?(@.d in [2, 3])]
$[?(2 in @.d)]
$[?(length(@) == 4)]
$[?(@.length() == 4)]
$[?(@.length == 4)]
$[?(@.key<42)]
$[?(@.key<=42)]
$[?(@.key='value')]
$[?(@.key*2==100)]
$[?(!(@.key==42))]
$[?(!(@.d==["v1","v2"]) || (@.d == true))]
$[?(!(@.key<42))]
$[?([email protected])]
$[?(@.a.*)]
$[?(@.key!=42)]
$[?((@.d!=["v1","v2"]) || (@.d == true))]
$[*].bookmarks[?(@.page == 45)]^^^
$[?(@.name=~/hello.*/)]
$[?(@.name=~/@.pattern/)]
$[?(@[*]>=4)]
$.x[?(@[*]>=$.y[*])]
$[?(@.key=42)]
$[?(@.a[?(@.price>10)])]
$[?(@.a.b.c==3)]
$[?(@.key-50==-100)]
$[?(@.key===42)]
$[?(@.key)]
$..[?(@.id)]
$[?(false)]
$[?(@..child)]
$[?(null)]
$[?(true)]
$[[email protected]==42]
$[?(@.key)]
$.data.sum()
$(key,more)
$..
$.key..
$
$
$
$[(@.length-1)]
$[0,0]
$['a','a']
$[?(@.key<3),?(@.key>6)]
$['key','another']
$['missing','key']
$[:]['c','d']
$[0]['c','d']
$.*['c','d']
$..['c','d']
$.*[0,:5]
$[1:3,4]
$[ 0 , 1 ]
$[*,1]
Replies to this thread might be a good place to collate tips on migrating JSONPath expressions.
The text was updated successfully, but these errors were encountered: