davtur19 / DotGit

An extension for checking if .git is exposed in visited websites
GNU General Public License v3.0
372 stars 32 forks source link

Background workers are not permanent in Manifest v3 #15

Open polcak opened 4 months ago

polcak commented 4 months ago

Hello,

it looks like you are using Manifest v3 and it looks like you store state in you background workers.

For example, let us focus on check_git https://github.com/davtur19/DotGit/blob/ab11f452c5dabc1cbb5488b67bda2f709dbeefb5/dotgit.js#L101. The extension writes to that variable in set_options (https://github.com/davtur19/DotGit/blob/ab11f452c5dabc1cbb5488b67bda2f709dbeefb5/dotgit.js#L101) and in the onMessage listener (https://github.com/davtur19/DotGit/blob/ab11f452c5dabc1cbb5488b67bda2f709dbeefb5/dotgit.js#L555). The extension reads the value in onHeaderReceived WebRequest API listener https://github.com/davtur19/DotGit/blob/ab11f452c5dabc1cbb5488b67bda2f709dbeefb5/dotgit.js#L714.

First of all, you are using a global variable and the code looks like it can produce race conditions for that variable if the onMessage listener is executed in short time with different content for the check_git variable. I have not checked if this is this can happen.

My main point is that the worker could have been destroyed between you store the value to the variable and onHeaderReceived fires. the code would broke if the variable should hold true. If I am looking right at the code, the variable would be undefined when the service worker respawns.

davtur19 commented 4 months ago

For now I'm not using manifestV3 yet, I was waiting for it to be supported by FireFox (I tried a few months ago and it didn't work) and I just did some testing on chrome and it seemed to work fine, until it's mandatory I don't think I'll make any changes unless necessary.

Anyway thanks for pointing out that in manifestV3 this might cause problems.

PR are welcome.