1

I was discussing about the subtle issue that you can face when you write in C, so (for fun) I started to create bulletproof code that can read an 32bit integer from stdin.

I wrote this code:

#include<stdint.h>
#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#include<sysexits.h>


void flush_stdin()
{   int c;
    while ((c = getchar()) != '\n' && c != EOF);
}


int32_t readInt32() 
{
    char line[13]; // -2147483648\n\0
    int32_t n = 0;
    if (fgets(line, sizeof(line), stdin)) {
        if (sscanf(line, "%d", &n) == 1) {
            if ((line[0] == '-' && n > 0) || (line[0] != '-' && n < 0)) {
                fprintf(stderr, "Overflow detected\n");
                exit(EX_DATAERR);
            }
            if (feof(stdin))
                flush_stdin();
            return n;
        }
    }
    fprintf(stderr, "Error: wrong int32\n");
    exit(EX_DATAERR);
}


int main() 
{
    int32_t r;
    r = readInt32();
    printf("read %d\n", r);
    return 0;
}

Is really bulletproof as I think? Do you have some tips about this code?

The main drawback is that it truncate the first 13 Byte of your input, so if you provide something like 000000000000001 it reads 0.

EDIT:

this code works as expected only if you generate a 64bit executable, but the solution provided by @Colin it's better...

Simone Aonzo
  • 165
  • 1
  • 1
  • 6

1 Answers1

3

I'm sure you can add more to secure it to make it the most secure, but another option could be to:

  1. Read in the value as an int64
  2. Check that the value is within the range min_int32 - max_int32
  3. if the value is within range the copy to an int32 variable, otherwise return an error condition.

Your code as it stands (and as you've checked for it) contains a possible integer overflow. This in itself is technically 'undefined behaviour', and as such, you cannot rely on the compiler or code execution from behaving.

Colin Cassidy
  • 1,900
  • 11
  • 19