#3 Remove Vulnerability in Search/Addmusic

Open
opened 3 years ago by tarfeef101 · 1 comments

Those commands can allow arbitrary host code execution, so we need to properly sanitize inputs and run these safely. This will also allow us to remove the whitelist. This was avoided for a long time because I didn't want to add syntax (delimiters) in multi-variable queries, but I think that'll be a necessary evil

Those commands can allow arbitrary host code execution, so we need to properly sanitize inputs and run these safely. This will also allow us to remove the whitelist. This was avoided for a long time because I didn't want to add syntax (delimiters) in multi-variable queries, but I think that'll be a necessary evil
tarfeef101 commented 3 years ago
Owner

since the underlying beets API still matches substrings (e.g. to get the song "confusion", you CAN submit the query "confusio"), we can leverage special characters we can expect to be rare, for users to often exclude by default, and wouldn't appear in the middle of a word.

Specifically, using a comma to separate variables in a query string should be okay, since most users won't put those in even if they are technically in a song/artist/album name, they won't be in the middle of words (causing their omission to lose results), and if we really want to, we could even check the word following one to see if the user is trying to use a command, or if they are continuing the last search string, since that will only fail in the miniscule edge case of a search string where the comma is before a word equal to a keyword, but the user intended to match the song/artist/album, not start a new variable in their search string.

since the underlying beets API still matches substrings (e.g. to get the song "confusion", you CAN submit the query "confusio"), we can leverage special characters we can expect to be rare, for users to often exclude by default, and wouldn't appear in the middle of a word. Specifically, using a comma to separate variables in a query string should be okay, since most users won't put those in even if they are technically in a song/artist/album name, they won't be in the middle of words (causing their omission to lose results), and if we really want to, we could even check the word following one to see if the user is trying to use a command, or if they are continuing the last search string, since that will only fail in the miniscule edge case of a search string where the comma is before a word equal to a keyword, but the user intended to match the song/artist/album, not start a new variable in their search string.
Sign in to join this conversation.
No Milestone
No assignee
1 Participants
Loading...
Cancel
Save
There is no content yet.