core: fix TOCTOU race condition

[szsam]

Separately checking the state of a file before operating on it may allow an attacker to modify the file between the two operations.

[msteveb]

Thanks. Technically you are right, but in practice if an attacker can modify or replace the file you are sourcing, you have bigger problems that either allocating too much memory if the size decreased or failing to read the entire file if the size increased.

[szsam]

I don't believe the existing code can detect whether the entire file is read. When sb.st_size is smaller than the actual file size, read() still succeeds.

jimtcl/jim.c

Lines 11650 to 11652 in 9784dcf

	readlen = read(fd, buf, sb.st_size);
	close(fd);
	if (readlen < 0) {

[msteveb]

That's true, but does it matter? If an attacker can modify or replace the file we can easily come up with simple attacks regardless. If the file is extended after stat but before read, why does it matter? It could just have easily been extended after read and then we wouldn't see it regardless? I just want to understand if there is any real scenario where this might cause a problem

[msteveb]

I would be incline to merge it anyway, since it doesn't hurt anything. But we don't rely on fstat() being available - hence bootstrap jimsh build fails.