Logging security

I recently got some security warning from a linter while logging user supplied data. I didn’t pay much attention at first because it’s not enabled in production and not even while I debug myself because there’s too much output. I only enable it when needed.

Anyway, the point is that you can fake log output. I didn’t believe it at first because there’s so many way to prevent that and the complex logging system I use (slf4j) would probably do that by default, right? Right?

So I added the following line to my program:

log.info("Completed\n2024-08-15T10:11:07.102+02:00  \u001B[33m" + "WARN\u001B[0m \u001B[35m35800\u001B[0m --- [JavaFX-Launcher] \u001B[36mio.xeres.app.application.Startup\u001B[0m         : System breach detected from ip 66.66.66.66. Computer terminated.");
Can you spot the fake line?

Three things to note:

  • you have to guess the correct time for the log, but this is easy enough
  • you have to guess the correct PID, this is harder but still possible, especially if the machine has been running for a long time and there’s already a log snippet somewhere
  • it’s easy for a system administator to miss those, so such a log might still induce panic and overblown response

I don’t know why loggers don’t strip ANSI sequences in user supplied data by default. This is dead easy and would actually bring a purpose to those ANSI colors!

I tried to find a setting to enable that but after 10 minutes I gave up. It’s not critical in my case anyway (I only log user supplied data for debugging). But still, it’s a point to remember.

#define is actually bad

A long time ago, when I switched from C to Java, I missed preprocessor macros, like:

#ifdef SOME_STUFF
...
#endif

This was annoying, especially on Android where there was no payment API yet, so the only way to make a paid application was to make a demo version alongside the paid one. Since both apps were essentially the same, you had to use some trickery to try to remove the functionality from the demo. Either a simple “if” condition, which could be easily changed by reverse engineering or trying to change the classes with some Gradle tricks.

Java has no preprocessor so you couldn’t do that. Back in the time I considered this as a drawback.

Well, turns out it’s actually a good thing. Because the main problem with conditional compilation is something I found out while trying to debug some C program recently:

Code rot within rarely used defines.

Indeed, I enabled some define to add additional logging and nothing worked: compilation errors everywhere. Why? Because the code around it changed and nobody bothered to check if the logging still worked.

Worse, sometimes when editing such code, the IDE (VSCode in that case) can become confused and show the wrong path since it cannot know for sure which define will be enabled or not, so you get some grey colored code which is actually the real one.

So, always remember that when you put code around a define, it’s no longer tested. The only legitimate use I see for them are for macros and for portability where you don’t have much choice anyway.

It’s all about maintenance

Programming is like sex: one mistake and you have to support it for the rest of your life.

— Michael Sinz

How would I guess, back when writing the first version of I’m sleeping,  that I would constantly need to come back to it with each release of a major Android version?

Even though Android has a mechanism which is supposed to eliminate such kind of burden, it wouldn’t work in this particular app’s case.

The purpose of the app is simple: wake at a precise time, twice per day to set or unset the volume of the phone’s ringer.

Easy enough! Simply use the framework’s AlarmManager:

alarmManager.set()

No, wait! Starting from Android KitKat (4.4), set() doesn’t guarantee precise timing anymore because of battery saving reasons. One has to use a new call:

alarmManager.setExact()

No, wait! In Android Marshmallow (6.0), setExact() doesn’t work when the device is in idle mode (ie. left alone on a table without moving), which means you’ll miss alarms. One has to use another new call:

alarmManager.setExactAndAllowWhileIdle()

The annoying thing is that each of these API changes would simply break my app if I wasn’t closely monitoring the framework’s evolution. I really hope there’s won’t be a setExactAndAllowWhileIdleAndDeepFreeze() or so call in a future revision of the framework.

This is a good example of why perfectly working and stable code still needs to be maintained because of all the changes around it (OS, drivers and libraries).

Update:

They did it again! From Android 12+, the following permission is required:

SCHEDULE_EXACT_ALARM

Enough is enough. I give up.

Using the proper data type

How to represent datatypes? For example in a JSON output.

Dates

Don’t do things like:

2014-01-17T18:01:30+01:00

Seriously. Do you have any idea how difficult and computationally intensive this is to parse? And I’m not even talking about the variants with timezones like GMT, PST, Z, +- offsets and so on.

Instead, do the following:

1389978090000

This is the number of milliseconds since the beginning of times, that is 1st January 1970 with an UTC time zone. It’s easy to parse and there’s no room for interpretation. Every library understands that format.

Durations

Don’t do

3111.44

This uses floats. Not only it can give rounding errors but it’s slower than integers. Instead do:

3111044

which is the number of milliseconds. Again easy to parse and every timer related library understands integer milliseconds, not necessarily floats.

Keep it simple and efficient.