jimtcl icon indicating copy to clipboard operation
jimtcl copied to clipboard

core: fix TOCTOU race condition

Open szsam opened this issue 2 years ago • 4 comments

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

szsam avatar Aug 23 '23 21:08 szsam

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.

msteveb avatar Aug 23 '23 23:08 msteveb

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. https://github.com/msteveb/jimtcl/blob/9784dcf88e8f0204550b4218f1c77bfa510a497b/jim.c#L11650-L11652

szsam avatar Aug 24 '23 02:08 szsam

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 avatar Aug 24 '23 03:08 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.

msteveb avatar Aug 27 '23 00:08 msteveb