ts: Make dostimestamp() fall back to epoch on parse failures

[syzzer]

If we detect corrupt timestamp data, fall back to the DOS epoch instead of passing on the ValueError raised by datetime(), which would stop the parsing run all together.

[Schamper]

There's a difference between a corrupt timestamp and an epoch timestamp though. Where does this currently break, perhaps that code should be more resilient? Or can you include a unit test for a corrupt timestamp?

[syzzer]

There's a difference between a corrupt timestamp and an epoch timestamp though. Where does this currently break, perhaps that code should be more resilient? Or can you include a unit test for a corrupt timestamp?

That would involve every user of dostimestamp() wrapping the call in a try/except block. That might arguably be more correct from a layering perspective, but seems quite error-prone and tedious.

In my case, the exception was passed on by dissect.target's filesystems/exfat.py:

        ctime = dostimestamp(fe.create_time, fe.create_offset).replace(tzinfo=c_tz)
        mtime = dostimestamp(fe.modified_time, fe.modified_offset).replace(tzinfo=m_tz)
        atime = dostimestamp(fe.access_time).replace(tzinfo=a_tz)`

dissect.fat currently seems to use the DOS Epoch as a fallback, which is why I did the same here. See e.g. these accessors in class DirectoryEntry:

    @property
    def ctime(self):
        if self.dirent.DIR_CrtDate and self.dirent.DIR_CrtTime:
            return dostimestamp(
                (self.dirent.DIR_CrtDate << 16) | self.dirent.DIR_CrtTime,
                self.dirent.DIR_CrtTimeTenth,
            )
        return datetime.datetime(1980, 1, 1)

    @property
    def atime(self):
        if self.dirent.DIR_LstAccDate:
            return dostimestamp(self.dirent.DIR_LstAccDate << 16)
        return datetime.datetime(1980, 1, 1)

So I took that approach and applied it for corrupted data too. If it helps, I can of course document that "DOS Epoch" is the fallback for unavailable or corrupted timestamps.

[Schamper]

Our ExFAT implementation hasn't been properly working for years and needs to be rewritten, so I wouldn't use that code as a reference for proper usage of this API. Perhaps the data it's trying to parse as a timestamp isn't even correct in the first place. As it currently stands I don't think this function should silently fallback to an epoch timestamp. In your current error case, I'd first start doubting the ExFAT code itself.

Doing this for this function also isn't in line with any of the other timestamp parsing functions. You'd want to at least log if an error occurred parsing a timestamp, but you don't have any useful context here on where or what timestamp actually failed to parse. That's information a consumer of this API has though, and could also decide on that the right course of action is (falling back on a default epoch, setting None, or breaking)