Security discussion

[SI-jvictors]

Hey Jamie, first off, really nice tool, Carl recommended it to me over Discord.

Anyway I have set it up on some internal hardware, hooked it into my GOES 16 and GOES 17/18 HRIT dishes, and scanned it with Burp Suite to check it out before I put it online, always a good thing to do. I noticed you wrote a little bit about security in your Readme, so I figured I'd check it out. I basically just fuzzed all the URLs used here, paying particular attention to the parameters of dataHandler.php. I threw a bunch of stuff at it and it seemed to check out. I also fuzzed each of the items in the currentSettings cookie, since that translates to the data returned by the "?type=weatherJSON" request.

Overall I sent about 240,000 requests and so far so good it seems. I always get nervous about PHP as there are a lot of gotchas and easy mistakes, but at least it seems to do proper data handling from what I could tell. Now it's just a basic fuzz, not a proper pentest, and I could have missed something, but at least it addressed my worries about low-hanging fruit. I think it's still fairly important to execute this software with an up-to-date server, current versions of PHP, and to use HTTPS if possible.

My only finding thus far is that the software seems to directly place the value of the "city" parameter in config.ini directly into the website. So for example if a user sets this to city = ABCD<i>hello</i> that HTML gets rendered in the web browser, giving a potential for HTML injection. I don't think this is a serious issue though, since only the admin would have edit permissions to config.ini (and if they were malicious they could just modify the software anyway) and while a user can also set this city themselves, they are just modifying their own cookie and thus attacking themselves. It doesn't seem like it's possible to set someone else's city with a CSRF attack because the cookie seems to be updated via Javascript, but you may want to verify that I didn't miss something there. In any case, consider running the "city" parameter and any other similar settings through an HTML sanitizer, such as PHP's htmlspecialchars function.

Anyway, I very recently set up https://goes.ruralradioprojects.com/ and put your software there. Work in progress, and I have to optimize it quite a bit, but so far I'm really liking it.

[JVital2013]

Thanks for checking out the code!

It's great to hear it stood up to your pentest - which sounds more robust than what I've thrown at it so far! All good advice on running up-to-date PHP, server, and to use HTTPS - although, I am slightly amused your site is running PHP 7.4. I guess you still have 8 days 😃.

Great find with the city parameter - the issue existed with the state parameter as well. I agree that it's not really exploitable, but best practices are best practices - I patched it as you suggested in a5b1a18.

You are correct that the currentSettings cookie is set, most of the time, via JavaScript. It is set by PHP on first load, and if PHP detects that the 1st "location profile" listed in the cookie is different from that configured in config.ini (ie. if the cookie was tampered with or config.ini on the server has changed).

[SI-jvictors]

No problem! As I said it was more of a fuzz, but it seemed pretty good. As an enhancement, I would also suggest a method of making the menu more obvious, like with a recognizable icon, some text, or a setting to make the menu permanently visible. Just an idea. I could file an issue if you want that organized there.

I am slightly amused your site is running PHP 7.4.

Yeah it's a shared server as I'm cheap. I'll see if I can poke support for upgrading. PHP is a quirky language and I want to make sure that it's current.

[JVital2013]

The menu icon change is a good idea - can you open an issue for it? I'll probably go with a hamburger menu with the satellite "superimposed" over the lower-right corner

[SI-jvictors]

Done, #29