Skip to content
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

Information leak: Last visit redirect is triggered also for anonymous requests (public home page, if shared document) #178

Open
almereyda opened this issue Aug 24, 2023 · 4 comments

Comments

@almereyda
Copy link

When an anonymous user visits an instance, they are redirected to the last visited page.

  • If that is a publicly shared page, they are stuck there.
    It is still possible to manually call the /login route for creating a valid session through logging in.
  • If it is a private page, they are redirected and asked to /login.

It would be expected that knowledge of the last_visit state is restricted to logged in users only, and not leaked to the public.


This runtime value is mutated when a logged in user does call loadNoteById:

const loadNoteById = useCallback(

And is set to the id for all cases, in which no new ID is opened:

if (!isNew && id !== 'new') {
await mutateSettings({
last_visit: `/${id}`,
});
}


This also allows to set a temporary home page of a Notea instance, by carefully choosing the page last navigated to in a Notea tab before closing it.


Further down the road, somehow a similar "feature", combining a targeted redirect and a publicly shared page, could allow for a nice and simple "home page" of an instance. This would seemingly require an additional (runtime) setting, such as last_visit, but set and named differently.


Remediation would probably center around these pieces of the code base, where the lastVisit is patched into the request, if a user isLoggedIn:

notea/pages/index.tsx

Lines 57 to 66 in d5f8113

const lastVisit = ctx.req.props?.settings?.last_visit;
if (lastVisit && ctx.req.session.get('user')?.isLoggedIn) {
return {
redirect: {
destination: lastVisit,
permanent: false,
},
};
}

This is always true, if the auth.type equals to none.

if (cfg.auth.type === 'none') {
return true;
}

Which always seems to be the case:

let auth: AuthConfiguration = { type: 'none' };

@almereyda
Copy link
Author

I'm also not perfectly sure how this line is to be read:

export type AuthConfiguration = { type: 'none' } | BasicAuthConfiguration;

And how it is to be interpreted in the context of the type exports just before:

export type BasicUser = { username: string; password: string };
type BasicMultiUserConfiguration = {
username?: never;
password?: never;
users: BasicUser[];
};
type BasicSingleUserConfiguration = { username?: string; password: string } & {
users?: never;
};
export type BasicAuthConfiguration = { type: 'basic' } & (
| BasicSingleUserConfiguration
| BasicMultiUserConfiguration
);

#108 states that Notea is distinctively a single user platform. Why would it declare and export any MultiUser type?

@almereyda
Copy link
Author

almereyda commented Aug 24, 2023

This is an edge case, which I have only found, because I had last visited a publicly shared page.

It's possible to reproduce this behaviour with an unauthenticated curl client:

$ curl -I https://notea.example.org 2>/dev/null | grep '307|location'
HTTP/2 307 
location: /U0lXWiHywE

@tecc
Copy link
Member

tecc commented Aug 24, 2023

This is actually a non-issue, at least in the latest alpha version. The issue was glanced upon in #169, which I promptly fixed.

The multiple-user configuration stuff is a remnant from when I was going to add that feature - this was when I was working on it as a fork, rather than as the maintainer. Things happened, I became the maintainer (although I've been doing an awful job thus far), and I decided to merge my changes to the upstream.

@almereyda
Copy link
Author

almereyda commented Aug 28, 2023

Thanks for fixing this already.

Yes, #169 (comment) describes the same issue, I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants