0

I have a function like this which aims to read a file:

int foo(FILE* f)

I want to use flock in order to prevent TOCTTOU. flock requires a file descriptor as an integer. I can get this using fileno(file). The implementation of foo therefore might look like this:

int foo(FILE* f) {
  if(!f) return -1;
  int fd = fileno(f);
  if(fd < 0) return -1;
  flock(fd, LOCK_EX);
  //do all the reading stuff and so on.
} 

However, the evil user might do something like this:

FILE* test;
test = fopen("someexistingfile.txt", "r");
fclose(test);
foo(test);

Then I have a problem because fileno will do invalid reads according to valgrind because it assumes that the file is open.

Any ideas on how to check whether the file is closed?

alk
  • 69,737
  • 10
  • 105
  • 255
今天春天
  • 941
  • 1
  • 13
  • 27
  • 3
    "*evil user*"? Well, I'd say "stupid" user. – alk Dec 30 '17 at 18:21
  • Check the return value of `flock()`. I strongly assume that it fails if `fd` is not a valid descriptor. – Martin R Dec 30 '17 at 18:24
  • @MartinR well problem is, `fileno` already gives invalid read if file is closed – 今天春天 Dec 30 '17 at 18:25
  • 1
    Related https://stackoverflow.com/q/21450109/694576 if not a duplicate to (see this answer https://stackoverflow.com/a/21450247/694576 to it). – alk Dec 30 '17 at 18:29
  • In particular file descriptors are opaque numbers that only mean anything to the kernel, if they are invalid (e.g. you just randomly pick a number and try to do an operation on it) it doesn't do anything. – Ahmed Masud Dec 30 '17 at 18:40
  • @AhmedMasud: "*check for fclose*" why? I mean how would this help to manoeuvre around the issue the OP is facing? – alk Dec 30 '17 at 18:41
  • Woopse fumbly fingers, I meant to type `flock` :P @alk, thank you for pointing it out. check for `flock` returning a -1 and check errno to see what went wrong. See `flock(2)` Return Value for details. It does set errno to -EBADF upon trying to lock an unopened file descriptor. – Ahmed Masud Dec 30 '17 at 18:51
  • @AhmedMasud as already pointed out in the post, `fileno()` is the problem because it causes invalid read...the problem is the invalid read. – 今天春天 Dec 30 '17 at 18:54
  • I'm wondering what kind of TOCTOU bug you're worried about. And there's another problem - a quite possible result of the undefined behavior of using `fileno()` on a closed `FILE *` is you get a file descriptor that refers to another file entirely. – Andrew Henle Dec 30 '17 at 20:14
  • I'm confused why this was downvoted. While OP has a fundamental misunderstanding of resource handles and why what they're asking for is impossible, I find it a *common misunderstanding*, and the question is stated clearly and reasonable to ask. – R.. GitHub STOP HELPING ICE Dec 30 '17 at 21:58

1 Answers1

3

C11 n1570 7.21.3p4

  1. A file may be disassociated from a controlling stream by closing the file. Output streams are flushed (any unwritten buffer contents are transmitted to the host environment) before the stream is disassociated from the file. The value of a pointer to a FILE object is indeterminate after the associated file is closed (including the standard text streams). Whether a file of zero length (on which no characters have been written by an output stream) actually exists is implementation-defined.

After fclose the use of the value of a FILE * in library functions leads to undefined behaviour. The value of the pointer cannot be used safely for anything at all until reassigned.

In other words, you cannot do really anything at all to discern whether the FILE * value you've given refers to a valid open file or not... well except for testing against NULL - if the value of the pointer is NULL it certainly cannot point to an open stream.