Skip to content

Using old asset URL exposes sensitiv information #6

@EternalBlack

Description

@EternalBlack

I for some reason would prefer one-time-download URLs as it provides a maximum level of security (would be nice, if this would be possible in the future) but for now, when I load my page with a "secure asset" I get a link like this:
"mydomain.com/secureAssetDownload/TCSGV4Q7RoBQQTQbgB7CVrtLoWJdCLBwhdV_7cSSun_nUlDr_vLDslQnvzAWCr5u_lZ0UHsZNLFO77plyamyvp76EKxXx7-vVh5KM-ymcJRBoNyE7WcLoXviLWbd6C0WTggcbE8rWQWhjVMVb33SoyXoLCgbEAhhk5Nv5jS9aTQ"
Now, when I change the secret key in the settings the URL of course changes as the hash changes, though if I still go to that old URL that is no longer valid I see something like this:

Internal Server Error

Argument 1 passed to Craft\SecureAssetDownloadService::isDownloadAllowed() must be of the type array, boolean given, called in /var/www/vhosts/mydomainprovider.com/mydomain.com/craft/plugins/secureassetdownload/controllers/SecureAssetDownloadController.php on line 11

which on the one hand is pretty ugly and on the other is a (for me) security risk as it provides mydomainprovider.com as well as the rest of the folder structure (beside letting the user know that I'm using Craft CMS). A 404 page or something would be nice here.

Also, if it's possible to you: An option to make it a one time download URL (that for example expires after 60min, to keep things clean) would be really nice and come in handy.

EDIT:
A 404 can be thrown with throw new HttpException('404', 'Hax');, currently trying to fix it myself, but need to dig a bit.
EDIT: "Fixed" it (at least I think so) by adding

if (gettype($options)!='array') {
	throw new HttpException('404', 'Hax');
}

to SecureAssetDownloadController.php right before the isDownloadAllowed check.

EDIT: To elaborate a bit more:
I would prefer the following setup:

{
  asset: id or AssetFileModel, // e.g 14 or craft.asset.id(23).first()
  userId: id or array of user ids, // e.g. 6 or [ 17, 28, 30 ]
  userGroupId: id or array of user group ids, // e.g. 3 or [ 1, 99, 76 ]
  forceDownload: true or false, // Optional field to force the download rather than opening in the tab (default: true)
  onetime: true or false, //optional defaults to false if not set
  duration: 60, // optional, defaults to global value in settings menu if not set
}

I'm currently not that sure what the best approach for this would be. There would need to be "On Asset Download Event" as well as either a timer or including the link creation time within the url (hashed), that get deciphered and interpreted as well on decryption as well as checked against.
Going to look into that, though I'm fairly new to Craft plugins in general, but still got some cryptographic and php knowledge.

Got it pretty much done, but it appears that I'm not yet able to make changes to the original options. Got to dig a bit more. I created a pull-request.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions