cdatabasestruct

Fprintf writing more than 1 member of struct, and excess data


When the program writes to the selected file (for now overwriting it) it also writes the next member in the struct

#include <stdio.h>
#include <string.h>
#include <sodium.h>
#include <tomcrypt.h>
struct user {
  char uName[32];
  char salt[16];
  char hash[32];
};
int main(int argc, char **argv){
  if (sodium_init() < 0) {
      return 1;
  }
  if(argc != 4){puts("Invalid number of arguments"); return 0;}
  struct user users[1];
  char passSalt[32 + 16];
  char hash[32];
  char salt[16];
  for(int i = 0; i < 16; i++){
    salt[i] = randombytes_uniform(127);
  }
  strcpy(passSalt, argv[3]);
  strncpy(passSalt, salt, 16);
  hash_state md;
  sha256_init(&md);
  sha256_process(&md, passSalt, 16);
  sha256_done(&md, hash);
  strncpy(users[0].uName, argv[2], 32);
  strncpy(users[0].hash, hash, 32);
  strncpy(users[0].salt, salt, 16);
  FILE *fp = fopen(argv[1], "w+");
  fprintf(fp, "%s %s %s\n", users[0].uName, users[0].hash, users[0].salt);
  fclose(fp);
}

Arguments used: ./write db abc qwertyuiop What is written to file: abc [ôÓBmdÂûÆ_LÂ¥ïÖ]lG31GfPmŽ@ÙûªŒU to~497s\0JmA[ôÓBmdÂûÆ_LÂ¥ïÖ]lG31GfPmŽ@ÙûªŒU

[ôÓBmdÂûÆ_LÂ¥ïÖ]lG31GfPmŽ@ÙûªŒU is written twice and @ÙûªŒU should not be written at all because the hash is only 32 bytes long


Solution

  • The %s format specifier writes a sequence of chars, up to a terminating null byte (which is not copied).

    Since your 'salt' is 16 random bytes, there may or may not be a terminating null byte, so fprintf may or may not run off the end.

    Similarly with 'hash'.

    You can use %.16sand %.32s to control this overrun, but the whole notion of printing non-text bytes as if they were text strings is flawed. For just one thing, if the salt happens to contain a zero byte, that's where 'printing' stops. Find a different approach.