Admin Pages are Open Access for Everybody with simple "Exploit"

[dremerb]

Applies to all pages in src/views/Admin/

Currently only presence of the isAdmin field in the JWT is checked, but the HMAC is never confirmed. By manipulating the token the admin pages are accessible. Absolutely not critical, as every action is validated in the backend and the user's permissions are checked independent from the token.

Affected Code:

kacky-eventpage-frontend/src/views/Admin/AdminIndex.jsx

Line 12 in 77302cc

if (!JSON.parse(atob(authentication.token.split('.')[1])).isAdmin) {

Thanks to janhenning on Discord for this one.

[Eriscot]

@dremerb For the SPAs, I can see only these ways to do it:

• Storing the secret key in frontend code since all the code is executed on the client side and the token needs to be checked against on the client side (doing so obviously screws the security) This is definitely a -- rating

• Sending the token to the backend to verify admin privileges before each call to Admin page (this may add some negligible impact on the backend, as we only need to do it only for Admin pages, but it'll be there, and also may require additional work on the backend)

If the latter seems an okay solution, I can implement that, but it's all up for the discussion

[dremerb]

Right, makes sense that verifying the HMAC needs the secret. Didn't really think about it after issue got raised.
That said, this is only half true - Token generation could be switched to asymmetric signing, then verification could be done with the public key. Saves a request, as proposed in your second idea.
The only downside would be that all users needs to sign in again - but tokens expire after 100 days anyway, so it only would become an issue for a handful of people.

On the other hand, I am still unsure if this needs a fix. Every permission verification in frontend can easily be bypassed in a local copy. And even a backend driven permission check would not solve the "issue" that API requests and page elements are available in the repository.
And even if the backend provides the page content, so it either returns "You are not an admin!" or some HTML to embed, the backend also is OSS and therefore nothing really changed.

So I think the real question is: Can this even be locked down in a way, that no unauthorized user will ever see it, while maintaining openness of code? Or would your second idea work in such a way?

[Eriscot]

Well, it depends. If we think of that as an OSS thing, this doesn't make sense, as you said, because anybody could just clone a repo, remove the check and view the pages. But if the only concern is the deployed application, the second option could work as you suggested (somehow I got confused by thinking asymmetric public keys are used to encrypt the data, not to validate the the right owner of the encrypted message, thanks for clarification).

[dremerb]

Well said.
Personally I kinda fail to see the benefit to build some elaborate system for this (apart from the fun doing it). I am very much on the "OSS thing" side. For the "deployed application" side, backend handles rogue requests and does it's own permission checking (someday you will see the code, I promise).

I'm down to listen to your call. If you want to wait for backend code and poke around once that's available, I'm happy to leave this open for now. If you think marking this as "Won't Fix" and moving on is perfectly fine, I see no issue with that as well.

To me this thing only really exists, because the user system got a "isAdmin" flag and this was the most trivial way to use it in Frontend. Once backend is updated and documented, no malicious user will spend much time with the frontend anyways - or they do and waste their time.

[Eriscot]

I'd advise to keep it open until either this is a concern that needs a fix or there's not much to do