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

Meteor Dev Tool mark users update insecure ?? #63

Open
koetheq opened this issue May 16, 2017 · 7 comments
Open

Meteor Dev Tool mark users update insecure ?? #63

koetheq opened this issue May 16, 2017 · 7 comments

Comments

@koetheq
Copy link

koetheq commented May 16, 2017

I like the Meteor Dev Tool. It helps me a lot in development. But the audit tool in MeteorDevTool mark my app insecure.

In my app, there is no allow/deny rules as recommended by meteor guide. And all user insert/update/remove are done through meteor method. check, audit-arguments-checks package are used, all argument check before execute. But when open MeteorDevTool =>security=>collections, click audit button of users collection, it still gives me a red update insecure message.

I read the article about how meteor dev tool determine a collection op is insecure, http://blog.thebakery.io/introducing-a-secury-auditor-to-meteor-devtools/, it says it will mark insecure if got an error other than 403... and says 'This means that some collections might be labeled as insecure even though the appropriate Allow/Deny rules have been setup and they present no immediate vulnerability.'

But this 'insecure' message makes developer and manager really scared . Therefore, I dig it further..

I looked into the DDP log and found the user update throws 400 Match Error instead of 403.
And the test is update with argument _{id:"invalid_id}
And in server console, such update op will throw an exception: Exception while invoking method '/users/update' Error: Match error: Expected object, got undefined
at exports.check (packages/check.js:57:15)
...

I tried some test method:
_Meteor.call('users/update', ({id:"invalid_id} )
==> throw 400 Match error (it will be marked as insecure by meteorDevTool)
_Meteor.call('users/update', ({id:"invalid_id},{})
==> throw 403 error ( it will be marked as secure by MeteorDevTool)

I tried tried similar update test direct to mongo in Robomongo:

_db.getCollection('users').update({'id':'invalid_id'} )
==> throw syntax exception
Error: need an object :
_DBCollection.prototype.parseUpdate@src/mongo/shell/collection.js:428:1
DBCollection.prototype.update@src/mongo/shell/collection.js:460:18

_db.getCollection('users').update({'id':'invalid_id'},{}})
==> pass: Updated 0 record(s) in 1ms ( no access check in Romomongo)

My impression:
The arguments for updating Meteor.users not allow only _id but no other field

If I am wrong, please enlighten me.
If I am right, could the MeteorDevTool could consider add some more dummy arguments to avoid wrongly insecure which scares us.

And also the collection meteor_autoupdate_clientVersions which is mark insecure for all insert/update/remove, even in a very simple app (a app created by 'meteor create app' and removed insecure, autopublish). What does that mean? what should we do?

Thanks!

Quan

@timmakken
Copy link

timmakken commented Aug 9, 2017

Im running into the same issues, all collections turn up green except the users collection update but i cannot find any evidence that this is actually true.

@callmephilip
Copy link
Member

Thanks for reporting this, guys. I am gonna take a look at this. @koetheq please don't get too anxious about this - looks like a bug in the tool 🌵

@sunlee-newyork
Copy link

+1

1 similar comment
@xanatas
Copy link

xanatas commented Dec 14, 2017

+1

@BramDecuypere
Copy link

  • 1

@MauriMatamoros
Copy link

+1

1 similar comment
@robinleboe-sw
Copy link

+1

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

8 participants