C
Backstory
My wife inherited a cat from family.† Unfortunately, I am very allergic to animals. The cat was well past its prime and should have been euthanized even before we got it, but she could not bring herself to get rid of it due to its sentimental value. I hatched a plan to end my its suffering.
We were going on an extended vacation, but she did not want to board the cat at the veterinarian's office. She was concerned about it contracting illness or being mistreated. I created an automatic cat feeder so that we could leave it at home. I wrote the microcontroller's firmware in C. The file containing main looked similar to the code below.
However, my wife is also a programmer and knew my feelings towards the cat, so she insisted on a code-review before agreeing to leave it at home unattended. She had several concerns, including:
main does not have a standards compliant signature (for a hosted implementation)main does not return a valuetempTm is used uninitialized since malloc was called instead of calloc- the return value of
malloc should not be cast
- the microcontroller time may be inaccurate or roll over (similar to the Y2K or Unix time 2038 problems)
- the
elapsedTime variable may not have sufficient range
It took a lot of convincing, but she finally agreed that theses weren't problems for various reasons (it didn't hurt that we were already late for our flight). Since there was no time for live testing, she approved the code and we went on vacation. When we returned a few weeks later, my the cat's misery was over (though as a result I've now got plenty more).
† Entirely fictitious scenario, no worries.
Code
#include <time.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
//#include "feedcat.h"
// contains extern void FeedCat(struct tm *);
// implemented in feedcat.c
// stub included here for demonstration only
#include <stdio.h>
// passed by pointer to avoid putting large structure on stack (which is very limited)
void FeedCat(struct tm *amPm)
{
if(amPm->tm_hour >= 12)
printf("Feeding cat dinner portion\n");
else
printf("Feeding cat breakfast portion\n");
}
// fallback value calculated based on MCU clock rate and average CPI
const uintmax_t FALLBACK_COUNTER_LIMIT = UINTMAX_MAX;
int main (void (*irqVector)(void))
{
// small stack variables
// seconds since last feed
int elapsedTime = 0;
// fallback fail-safe counter
uintmax_t loopIterationsSinceFeed = 0;
// last time cat was fed
time_t lastFeedingTime;
// current time
time_t nowTime;
// large struct on the heap
// stores converted calendar time to help determine how much food to
// dispense (morning vs. evening)
struct tm * tempTm = (struct tm *)malloc(sizeof(struct tm));
// assume the cat hasn't been fed for a long time (in case, for instance,
// the feeder lost power), so make sure it's fed the first time through
lastFeedingTime = (size_t)(-1);
while(1)
{
// increment fallback counter to protect in case of time loss
// or other anomaly
loopIterationsSinceFeed++;
// get current time, write into to nowTime
time(&nowTime);
// calculate time since last feeding
elapsedTime = (int)difftime(nowTime, lastFeedingTime);
// get calendar time, write into tempTm since localtime uses an
// internal static variable
memcpy(&tempTm, localtime(&nowTime), sizeof(struct tm));
// feed the cat if 12 hours have elapsed or if our fallback
// counter reaches the limit
if( elapsedTime >= 12*60*60 ||
loopIterationsSinceFeed >= FALLBACK_COUNTER_LIMIT)
{
// dispense food
FeedCat(tempTm);
// update last feeding time
time(&lastFeedingTime);
// reset fallback counter
loopIterationsSinceFeed = 0;
}
}
}
Undefined behavior:
For those who don't want to bother finding the UB themselves:
There's definitely local-specific, unspecified, and implementation-defined behavior in this code, but that all should work correctly. The problem is in the following lines of code:
struct tm * tempTm //...
//...
memcpy(&tempTm, localtime(&nowTime), sizeof(struct tm));
memcpy overwrites the tempTM pointer instead of the object it points to, smashing the stack. This overwrites, in addition to other things, elapsedTime and loopIterationsSinceFeed. Here's an example run where I printed out the values:
pre-smash : elapsedTime=1394210441 loopIterationsSinceFeed=1
post-smash : elapsedTime=65 loopIterationsSinceFeed=0
Probability of killing the cat:
- Given the constrained execution environment and build chain, the undefined behavior always occurs.
- Similarly, the undefined behavior always prevents the cat feeder from working as intended (or rather, allows it to "work" as intended).
- If the feeder does not work, it is extremely likely the cat will die. This is not a cat that can fend for itself, and I failed to ask the neighbor to look in on it.
I estimate that the cat dies with probability 0.995.
catcommand or something of that sort. \$\endgroup\$