-
Notifications
You must be signed in to change notification settings - Fork 41
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
unquoted note body fails #80
Comments
Hi, isn't this what should already happen in https://github.com/Red5d/pushbullet-bash/blob/master/pushbullet#L390 ? |
Does it work for you? |
No, your example breaks for me as well, but the problems are not the quotation marks, but rather the escaping of those. As |
Oh, I think I see ... interesting, that isn't ultimately what I was getting at, but I didn't realize the underlying issue. My thought is that one could include anything that is html-encodeable, to include backslashes and single-/double-quotes. Instead of removing all unprintable characters, what do you think about taking the string and just shell-escaping it? Perhaps something like this For me the point is not so much unprintable chars (though I can see value in that for many) but that I think it's useful to send arbitrarily-quoted strings. (Ok, not arbitrary, it's well-structured, but you get the point.) |
Not a huge surprise, but notes with quotes in the body break sending notes:
This can be averted with something like the following (which includes the posix-ification from my posix PR):
body=$(echo "$body" | sed -e 's/\n/\\n/g' -e 's/"/\\"/g')
Though I haven't come up with a string that would constitute an injection attack, it seems prudent to guard against inadvertent interpretation. (I'm doing this because I find it handy to have easily parsed message bodies; whether using CSV or JSON in the message body, it's not hard to conceive of situations where having some components quoted might be useful.)
There may be other shell escaping that might be necessary; I haven't investigated possible problems yet. Likewise, it might be a good idea to do this for the note title as well.
The text was updated successfully, but these errors were encountered: