Question about security

[marcreichel]

Hi,

I'm just getting started with this package but I have one security related question:

When brute forcing the IDs of a wizard it is possible to access pending wizards (from other sessions) which might have stored sensitive data already (like names, addresses or email addresses for example). Is it possible to prevent this behavior?

An example:

• User A starts a wizard (ID 1) and enters his home address and email address in step 1 and saves.
• User B opens /wizard/{slug}/1/sensitive-step in his browser and sees the data of User A.

In my opinion the data should be bound to the session (and not to an URL parameter) or the ID should be stored in an encrypted cookie for example if there's no other solution with the current implementation via URL.

Let the discussion begin 😄🚀

[ksassnowski]

Hey, thanks for starting this discussion. I’m currently sitting at the airport without my laptop but I’ll get back to this over the weekend.

[bfiessinger]

Apart from the Session based variant I recently pushed I just came up with the idea doing the same with a cookie based variant.

This would combine the advantages of securing sensitive data and the ability to close the wizard and come back later. @ksassnowski what do you think?

[ksassnowski]

So this isn’t really a question of security but one of (lacking) documentation. Let me explain what I mean.

There’s nothing inherently unsafe about a wizard being accessible by anyone who knows the URL. It’s only a problem if your application has a requirement that wizards can only be viewed by certain users. Here’s the thing, though. This is true of every route in your application. Laravel doesn’t automatically restrict access to a route just because you named it users/{id}. You have to do that yourself, either via middleware, policies or some other means. Or maybe the route should be open because it links to the user’s public profile. There’s no way to know. It’s an application concern, not a package or framework concern.

Arcanist is no different in this regard. If a wizard should only be accessible by the user who created it, it’s your responsibility to ensure that happens. There’s no way Arcanist can know what your access control looks like. My original use case for writing this package explicitly required that multiple users (from the same organization) be able to access the same wizard.

This is where the documentation comes in, or lack thereof. It’s not immediately obvious how to add any kind of access control for wizards. If there was a section in the docs that explicitly said “Look, by default Arcanist allows everyone to access any wizard as long as you know the URL. If this is not what you want, here’s a reference implementation of how you can limit who can access a wizard”, then I don’t think this issue would’ve come up.

You basically discovered one of the many ways of how to do this yourself: you implemented a custom repository that automatically scopes wizards (on the user’s session in your case). This is great if you want to ensure that a wizard can only ever be accessed by the user who created it. This obviously wouldn’t have worked for my original use case, however.

Another way could be to include a hidden field with the current user’s id (or any other identifier) in the first step and save this as part of the wizard’s data. You could then write a middleware that checks if the current user matches the wizard’s user id. You could then either register the middleware globally via the arcanist.middleware setting or on a per-wizard level by overriding the static middleware method on the wizard class.

I will leave this issue open for posterity but I don’t really consider it a problem with the code. Version 1 of Arcanist (release date unknown) will be a complete rewrite anyways because I’m learning about all the different ways people are using and trying to extend this package.

[marcreichel]

@ksassnowski Thank you for your detailed response. 👍🏼 Really appreciate it! I've implemented the session based wizard repository of @bfiessinger in my application and it works just how I need it.

Looking forward to the further development of Arcanist 👍🏼

[ziming]

Hmm maybe the wizards database table id should be uuid to make it harder to guess. I agree it can be risky if the person using this package is not aware.

[ksassnowski]

Again, relying on a resource identifier being hard to guess is not a valid security strategy. If wizards should be restricted to certain users, this should actually be enforced by the application.

[ziming]

yes but i think many people using this package may not even be aware of this id guessing security issue, so if the default choices are more secure for them (Such as the SessionWizardRepository PR) it could be better

That's also how why by default template language like blade escape the strings

In fact I myself missed it too until I saw this issue.