pushbullet-bash icon indicating copy to clipboard operation
pushbullet-bash copied to clipboard

unquoted note body fails

Open r2evans opened this issue 7 years ago • 4 comments

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.

r2evans avatar May 07 '17 05:05 r2evans

Hi,

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

fbartels avatar May 08 '17 14:05 fbartels

Does it work for you?

r2evans avatar May 08 '17 14:05 r2evans

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

fbartels avatar May 08 '17 15:05 fbartels

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

r2evans avatar May 08 '17 16:05 r2evans