Replies: 13 comments 29 replies
-
Nice explanation. I was just looking at some code and I do a |
Beta Was this translation helpful? Give feedback.
-
@vickkhera yes, that would mitigate it as well. setSession calls getUser under-the-hood, and then saves a subset of data to /* Reference: https://github.com/supabase/auth-js/blob/master/src/GoTrueClient.ts#L1328-L1334 */
{
access_token, /* Whatever was passed-in to setSession() */
refresh_token, /* Whatever was passed-in to setSession() */
user, /* Based on the getUser() call */
token_type, /* Always "bearer" */
expires_at, /* Calculated from the decoded access_token */
expires_in /* From the decoded access_token */
} |
Beta Was this translation helpful? Give feedback.
-
Thank you for this explanation @j4w8n. The team is actively working on new features within auth-js that makes it easier to verify the JWT (access token) soon. This should alleviate some of the problems with latency if you rely on
A leaked JWT secret opens up not only your users under attack (as they can be easily impersonated) but more severely your database (the secret can be used to mint your own JWTs with any role that can bypass RLS). |
Beta Was this translation helpful? Give feedback.
-
@j4w8n I have a simple question on this.
Is this only an issue though if there are "trusted" operations performed server side or server component side? If one performs queries for example in a server component and implements @supabase/ssr the mal formed attacker cookie would be passed to the server side supabase instance for example in a middleware in next.js or passed to an edgefunction that generate a server based client with it. For the middle ware In the old version in of the supabase docs (from a couple weeks ago) this still used If we stick to the old version
in an edge function same by reading the auth header from the request and passing that into the create client
this would use the mal formed cookie now and not verify it. But if one now performs a database query with this mal formed supabase client the query should fail on the backend as the backend would try to validate the malformed jwt that is send with the request? |
Beta Was this translation helpful? Give feedback.
-
Can I ask, other than |
Beta Was this translation helpful? Give feedback.
-
I am trying to build my app with Next.js and Supabase Auth following this doc: https://supabase.com/docs/guides/auth/server-side/nextjs Now the doc says we should use But The Next.js middleware runs on edge runtime, which is close to end users but may be farway from Supabase Auth server Then, the In my case, the latency is more than 500ms,from HongKong to the US.
I hope you guys can solve this problem as soon as possible, may be you can issue a signed idToken and read user profile from it. |
Beta Was this translation helpful? Give feedback.
-
I implemented the new getSafeSession changes in my project and had no indication locally that there would be a massive spike in redundant auth requests caused by this. Once I deployed, I eventually noticed a huge spike and thought I was getting DDoS'd... I immediately jumped from <100 auth requests per hour to around 1-3k per hour. My project uses Sveltekit, but I noticed @Zanzofily next.js safesession package created to address the performance problems as well. He notes that one user generated 1,685 calls. I implemented this change based on the official docs. I imagine this issue is widespread, but I'm still seeing stale Git issues from back in March. I created a reddit thread for more visibility. Now I'm scrambling to implement the changes based on your sveltekit-supabase-ssr project. |
Beta Was this translation helpful? Give feedback.
-
Amazing article! Edit: //supabase middleware creation
const { data: activatedUser, error } = await supabase.auth.getSession();
return NextResponse.rewrite(new URL("/login", request.url));
}
return response; While any other request I make using the user's actual data(i.e. the user's ID) is always received using getUser Also due to the getSession that is used(although no data is used to fetch anything) - I get the log that it's unsafe to use that which is fair as it is but calling getUser for each server action that is used in the client(they all go through my middleware) seems extensive as I also have RLS setup and most of the time for me calling getUser causes rate limits |
Beta Was this translation helpful? Give feedback.
-
It looks like JWT Secret usage may be quite sensitice. It i is shame to need to use another secrete and take care of it. |
Beta Was this translation helpful? Give feedback.
-
I created a safe workaround to verify from JWT using Supabase JWT Secret. Source: https://gist.github.com/anarkrypto/9ee98bd23f25efd44a4cfb4ed256837a Usage: const safeSession = new SupabaseSafeSession(supabaseInstance, jwtSecret)
const { data, error } = await safeSession.getUser()
if (error) throw new Error(error.message)
console.log("user data":, data) Result: {
"id": "12345678-90ab-cdef-1234-567890abcdef",
"session_id": "fedcba98-7654-3210-fedc-ba9876543210",
"role": "authenticated",
"email": "[email protected]",
"phone": "",
"app_metadata": { "provider": "email", "providers": ["email"] },
"user_metadata": {
"email": "[email protected]",
"email_verified": false,
"phone_verified": false,
"sub": "12345678-90ab-cdef-1234-567890abcdef"
}
} |
Beta Was this translation helpful? Give feedback.
-
A few questions to check my sanity, since some how this sounds "worse" than it actually is.
The expectation by whoever added the warning was that it should be CRYSTAL CLEAR that using service_role keys on the client side and circumventing RLS is a recipe for disaster. Or is there some specific safety on the client that makes this "ok" there that I simply can not see? So, again, this attack vector exists only in connection with using RLS-circumventing-queries anywhere, client or server side, or displaying data from some non-supabase source based on the session.user data. Actually, if you never USE for example the user uuid anywhere, or some special metadata from the JWT to unlock features which interact with places outside of supabase, and use RLS in supabase, there is no need to run getUser() on every request since any interaction with supabase will also return an authentication error of some sort if the data was compromised or faked, or the site will simply not work with compromised or faked data. Of course I am very much looking forward to asymmetric JWTs being implemented (and btw, they are implemented in supabase/auth for a couple of weeks now, thanks km (if I remember correctly)..., I am guessing work is being done on the UI and the other libraries to support it as we speak), simply because it is nice to be able to authenticate the JWT safely anywhere, including on the client, and knowing one can depend on the data inside the JWT just like one could on server-stored session data back in the old days, prior to JWTs and client side javascript trickeries. But, in no way are we forced to depend on the data today, OR are we forced to run getUser() on every single interaction with our system, as long as RLS is used the way it should be. Just wanted to put this into perspective and make sure I am not missing something... |
Beta Was this translation helpful? Give feedback.
-
Any news on this? |
Beta Was this translation helpful? Give feedback.
-
UPDATE: I've been able to replicate this attack on the client side. This is because an attacker can easily sign up for an account on your app, then tweak their own cookie value to have
session.user.id
be the victim's user id. Therefore, it's value can't be trusted from a client sidegetSession()
call either - in my opinion. See supabase/auth-js#935As briefly described on PR supabase/auth-helpers#722, an attacker can retrieve sensitive user data through an attack vector within auth-js and a developer's code. This attack is based on a developer:
session.user
In this discussion, I'd like to clarify the topic, from my perspective, and point out some solutions and pitfalls. I welcome your thoughts.
To summarize:
getSession()
on the server and then use unique data likesession.user.id
, to return that user's sensitive information.getSession()
to retrieve a user's tokens (access, refresh).getSession()
is likely safe, but be cautious - e.g. a public url for their avatar, viasession.user.user_metadata.avatar_url
in the case of a GitHub OAuth login. Just keep in mind that accessingsession.user
currently throws a console warning.getSession() vulernability
If we look at how this method works, we see that it runs minimal checks on the potential session it grabs from storage. When I say "minimal checks", I'm not casting any blame; Supabase can't do much more than what it already is.
null
.object
, and that it has the propertiesaccess_token
,refresh_token
, andexpires_at
.expires_at
is some point in the future, it returns the session to you.This seems reasonable, but if we craft a cookie with the following values, it would pass all of those checks:
Developer code
Say you're server-side rendering a user's page that's been requested. You check that the user is logged-in by calling
getSession()
- perhaps in middleware. Data from that call comes back to you, so it seems we have an authenticated user for this request. So, now you fetch the user's sensitive data based on the value ofsession.user.id
, then the page contents are returned and rendered in the browser.It's worth noting that if you're creating a typical Supabase client that's been authenticated with the user's
access_token
, you're likely not going to fall victim here. This is because any db calls are going to be using that access token against your RLS policies - assuming you use something likeauth.uid()
. Even if you were to have a request like.eq('user_id', session.user.id)
, it would fail RLS and no data would be rendered; because the access token's user won't have permissions for any table rows for your victim-user's id.I'd say the most likely scenario for an attack would be if you're using a "service role" client, or other Supabase client which has elevated db permissions or can bypass RLS (or possibly an RPC call). Because in this scenario, you might craft code similar to:
In which case...
Attack!
For this attack to work, we need to assume the attacker was able to acquire a victim-user's Supabase user id. It also requires knowledge of the cookie name for user sessions, but this is trivial to find. We also assume the attacker does not have the user's access token or refresh token - otherwise they could just use those to get the victim's data.
Now imagine the attacker sends a page request to the server, using a properly-named cookie with this data:
Oh snap! All of the
getSession()
checks have passed and you've now sent the victim-user's sensitive data back to the attacker.Mitigation
!!!!!
Only use your JWT secret on the server-side, and only via a private env var. If there are other secure methods, I don't know what they are.
!!!!!
Here are a few things we can do to prevent this attack.
session
to render sensitive data. The only exception is verifying and decoding theaccess_token
, with a library such as jsonwebtoken, and then use information within the decoded JWT to load sensitive user data. So, from our example, instead of usingsession.user.id
, grab the value of thesub
property from a verified and decoded JWT - which is the user id of whoever the JWT was created for.getUser()
to retrieve the user's data and use it to fetch further data for the page. Yes, the getUser call grabs theaccess_token
from getSession and verifies it, but it's worth mentioning that you still can't trust anything else from thesession
info returned bygetSession()
. Anyway, if step 1 is not feasible or desired, calling getUser is the easiest solution. The downside is that it requires an extra network request every time you call it - either to Supabase's cloud instance or your self-hosted Supabase instance.False Security
When I came across this attack vector, my natural solution was to simply call
getSession()
and verify the JWT, akasession.access_token
. I assumed that as long as the JWT checks out, I can use data withinsession.user
to render the page.However, after thinking about it more, I realized this is a mistake. To understand why, imagine an attacker signs up for an actual account with your app and signs-in. They can then take their legit access token and send a separate request with the below cookie data:
As you can see, this would pass the
getSession()
checks and your JWT verification (access_token), but would still return the victim-user's data. So don't assume everything inside ofsession
can be trusted just because you were able to verify the access_token.If, instead, we were to grab the legit attacker's token and use the decoded JWT info from it, this would render the attacker's data instead of the would-be victim's. Just take precautions to only use your project's JWT secret, to verify/decode, on the server-side.
Beta Was this translation helpful? Give feedback.
All reactions