core icon indicating copy to clipboard operation
core copied to clipboard

Monit: support quotes in passwords

Open fbrendel opened this issue 2 years ago • 7 comments

Passwords containing single quotes must be bounded by double quotes and vice versa. So passwords with double quotes needs to be bounded by single quotes. See #6748

fbrendel avatar Jan 22 '24 10:01 fbrendel

And if it has both? ;)

fichtner avatar Jan 22 '24 10:01 fichtner

Then we could encode it. 123'"456 becomes '123\0x27\0x22456' But wait, what if the password contains the sequence \0x22 ?

fbrendel avatar Jan 22 '24 12:01 fbrendel

Highly unlikely but could happen. However I think it's better than the case before, where we don't handle double quotes. I had rough couple of hours trying to figure out why my monit setup don't send anything out then I discovered my password contains some quotes and it broke monit config. However if I was informed or got prevented from saving that password in the web page setup form, none of this would've happened. Can we modify the web form so it prevents certain stuff?

MAkcanca avatar Jan 22 '24 13:01 MAkcanca

I want to be very honest: security "researchers" don't give a damn about probability and this is clearly problematic from a technical point of view. Touch this once and fix all the issues is the best approach or else you will end up spending more time and more time again and processing a CVE eventually...

That being said single and double quotes have different escaping rules. In the shell you can use different quotes to write problematic quotes. I don't know how this works for monit but I also don't want to investigate.

"' can be escaped as '"'"'" in the shell, but bison probably uses escapes here so it needs to be checked which escape sequences are less for which quote and pick that as a standard and only deal with the remaining offending quote char separately (and escape the escape character by default, so \ becomes \\).

Cheers, Franco

fichtner avatar Jan 23 '24 08:01 fichtner

@fichtner But monit doesn't use shell to parse? Maybe I'm missing the point. See https://github.com/arnaudsj/monit/blob/aa491f6ca2d2dd79abebf1e97b6f3a14fd4453e8/util.c#L396 https://github.com/arnaudsj/monit/blob/aa491f6ca2d2dd79abebf1e97b6f3a14fd4453e8/l.l#L408

But if you can make it work, that would be awesome. Good luck!

MAkcanca avatar Jan 23 '24 14:01 MAkcanca

I'm only explaining my point because the approach so far is not cutting it.

fichtner avatar Jan 23 '24 14:01 fichtner

Ok, then, to be on the safe site, we have to encode all charachters. @fichtner is it possible to do that via a new field type e.g. HexField? The idea is to store the whole password as hex string which is compatible with the Monit syntax.

fbrendel avatar Jan 24 '24 09:01 fbrendel