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

unquoted note body fails #80

Open
r2evans opened this issue May 7, 2017 · 4 comments
Open

unquoted note body fails #80

r2evans opened this issue May 7, 2017 · 4 comments

Comments

@r2evans
Copy link
Contributor

r2evans commented May 7, 2017

Not a huge surprise, but notes with quotes in the body break sending notes:

root@localhost# ./pushbullet push mydev note "msg" "hello world"
Sending to device MYDEV
root@localhost# ./pushbullet push mydev note "msg" "hello \"world\""
Sending to device MYDEV
Error submitting the request. The error message was: {"error":{"code":"invalid_request","type":"invalid_request","message":"Failed to decode JSON body.","cat":"(=^‥^=)"},"error_code":"invalid_request"}

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.

@fbartels
Copy link
Collaborator

fbartels commented May 8, 2017

Hi,

isn't this what should already happen in https://github.com/Red5d/pushbullet-bash/blob/master/pushbullet#L390 ?

@r2evans
Copy link
Contributor Author

r2evans commented May 8, 2017

Does it work for you?

@fbartels
Copy link
Collaborator

fbartels commented May 8, 2017

No, your example breaks for me as well, but the problems are not the quotation marks, but rather the escaping of those. As tr also removes other unprintable chars, I would not remove it from the code, but rather extend the current line to also replace/remove \.

@r2evans
Copy link
Contributor Author

r2evans commented May 8, 2017

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 sed script: http://stackoverflow.com/a/20053121/3358272 (the last sed command, not the backslash-everything one).

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.)

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

2 participants