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.

Sign up for GitHub

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

Jump to bottom

What is the danger of having the . operator in queries? #36

Closed
angelov-a opened this issue Jan 8, 2021 · 5 comments
Closed

What is the danger of having the . operator in queries? #36

angelov-a opened this issue Jan 8, 2021 · 5 comments

Comments

@angelov-a
Copy link

I have only been able to find exploits using the $ operator. On the other hand using the dot is useful for accessing nested properties.

Can you please elaborate?
@fiznool
Copy link
Owner

fiznool commented Jan 12, 2021

Good question.

Once upon a time, it was not possible to use periods in MongoDB property names. When this library was first conceived, one of the use cases was to pass a property name directly to the server via the POST (JSON) body. It was decided to explicitly disallow any value which would have caused a problem when used as a key in a MongoDB operation - this included properties beginning with $ or containing ., as per this Jira ticket from the MongoDB issue tracker.

So, disallowing a . was less of a security concern, but it was still necessary to ensure that user-supplied data could be used in a MongoDB operation.

Modern versions of Mongo may well allow periods in the property names, I'm not sure what the status of this is? In any case, I'd be wary of changing this behaviour for fear of breaking code that already depends upon this behaviour.

@angelov-a
Copy link
Author

Thanks for the answer.

Maybe it's possible to add another option? This will prevent from breaking the current behaviour.

interface Options {
    replaceWith: string;
    allowDots?: boolean;
}

@fiznool
Copy link
Owner

fiznool commented Jan 13, 2021

Sounds good. I'd be happy to accept a PR which implements this, along with tests and documentation! 🙂

Blagoj5 added a commit to Blagoj5/express-mongo-sanitize that referenced this issue Jan 23, 2021
@Blagoj5
feat: Adding new options (options.allowDots)
aec9249 
Adding new option/feature, options.allowDots that is used for skipping the sanitization of data that has .(dot). This can be useful for nested document quering for mongoDb: https://docs.mongodb.com/manual/tutorial/query-embedded-documents/

Creating new tests that include the new option

Updating the documentation (README.md) file for the new option

Adressing issue: fiznool#36
@Blagoj5 Blagoj5 mentioned this issue Jan 23, 2021
Closed
@Blagoj5
Copy link

Blagoj5 commented Jan 23, 2021 •
edited
Loading

Hello @fiznool , i added the new options, tests and updated the docs-> README.md file. I would appreciate if you check the pull requests.

EDIT:
FIXED

Blagoj5 added a commit to Blagoj5/express-mongo-sanitize that referenced this issue Jan 23, 2021
@Blagoj5
feat: Adding new options (options.allowDots)
287075b 
Adding new option/feature, options.allowDots that is used for skipping the sanitization of data that has .(dot). This can be useful for nested document quering for mongoDb: https://docs.mongodb.com/manual/tutorial/query-embedded-documents/

Creating new tests that include the new option

Updating the documentation (README.md) file for the new option

Adressing issue: fiznool#36
Blagoj5 added a commit to Blagoj5/express-mongo-sanitize that referenced this issue Jan 23, 2021
@Blagoj5
feat: Adding new options (options.allowDots)
05e39bb 
Adding new option/feature, options.allowDots that is used for skipping the sanitization of data that has .(dot). This can be useful for nested document quering for mongoDb: https://docs.mongodb.com/manual/tutorial/query-embedded-documents/

Creating new tests that include the new option

Updating the documentation (README.md) file for the new option

Adressing issue: fiznool#36
@Blagoj5 Blagoj5 mentioned this issue Jan 23, 2021
Closed
fiznool added a commit that referenced this issue Jan 14, 2022
@fiznool
Adding options.allowDots (#41)
32c997b 
Squashed commit of the following:

commit f85e51644ed68a74dd5fbe7a79c17e62e02aed01
Author: Tom Spencer <[email protected]>
Date:   Fri Jan 14 10:47:41 2022 +0000

    Removed unnecessary file

commit 059be6ba7fb8a4d6e80147a94b03a064f1a43fcc
Merge: 565c1ea 55a16c7
Author: Tom Spencer <[email protected]>
Date:   Fri Jan 14 10:46:46 2022 +0000

    Merge branch 'master' of github.com:Blagoj5/express-mongo-sanitize into Blagoj5-master

commit 55a16c7
Merge: 16534f2 9cc5240
Author: Blagoj <[email protected]>
Date:   Wed May 12 18:22:57 2021 +0200

    Merge github.com:fiznool/express-mongo-sanitize

commit 16534f2
Author: Blagoj <[email protected]>
Date:   Wed May 12 17:07:35 2021 +0200

    Clean code and fix tests

commit 565c1ea
Author: Tom Spencer <[email protected]>
Date:   Tue May 11 16:47:57 2021 +0100

    Bump package version to 2.1.0

commit 05e39bb
Author: Blagoj <[email protected]>
Date:   Sat Jan 23 16:40:25 2021 +0100

    feat: Adding new options (options.allowDots)

    Adding new option/feature, options.allowDots that is used for skipping the sanitization of data that has .(dot). This can be useful for nested document quering for mongoDb: https://docs.mongodb.com/manual/tutorial/query-embedded-documents/

    Creating new tests that include the new option

    Updating the documentation (README.md) file for the new option

    Adressing issue: #36

commit 287075b
Author: Blagoj <[email protected]>
Date:   Sat Jan 23 16:38:15 2021 +0100

    feat: Adding new options (options.allowDots)

    Adding new option/feature, options.allowDots that is used for skipping the sanitization of data that has .(dot). This can be useful for nested document quering for mongoDb: https://docs.mongodb.com/manual/tutorial/query-embedded-documents/

    Creating new tests that include the new option

    Updating the documentation (README.md) file for the new option

    Adressing issue: #36

commit aec9249
Author: Blagoj <[email protected]>
Date:   Sat Jan 23 16:17:53 2021 +0100

    feat: Adding new options (options.allowDots)

    Adding new option/feature, options.allowDots that is used for skipping the sanitization of data that has .(dot). This can be useful for nested document quering for mongoDb: https://docs.mongodb.com/manual/tutorial/query-embedded-documents/

    Creating new tests that include the new option

    Updating the documentation (README.md) file for the new option

    Adressing issue: #36
@fiznool
Copy link
Owner

fiznool commented Jan 14, 2022

allowDots option has now been included, closing as this is now resolved.

@fiznool fiznool closed this as completed Jan 14, 2022
@TeemoSasuke TeemoSasuke mentioned this issue Sep 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fiznool @angelov-a @Blagoj5