Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

qBittorrent not checking if destination file exists when using Skip Hash Check. #11841

Open
an0n666 opened this issue Jan 8, 2020 · 19 comments
Open

Comments

@an0n666
Copy link
Contributor

@an0n666 an0n666 commented Jan 8, 2020

Please provide the following information

qBittorrent version and Operating System

(type here)
4.2.1, W10

If on linux, libtorrent-rasterbar and Qt version

(type here)

What is the problem

(type here)
Qbit is not checking if destination file exists when using skip hash check. It directly goes into seeding mode even if you select a directory which doesn't contain the torrent's files. One could select a wrong directory when adding a file for re-seeding or cross-seeding and qBit won’t complain that the files are missing. When a leecher connects and read piece is called, libtorrent knows file doesn’t exists so it starts downloading it again. This is an unexpected behaviour.

What is the expected behavior

(type here)
It should call read_piece() in libtorrent or at least check if the file exists before going into seeding mode.

Steps to reproduce

(type here)
Just add some torrent in qBit and mark skip hash check (make sure that torrent has no leecher. Otherwise when a piece read is called it will reset back to 0% and start downloading).

Extra info(if any)

(type here)

@an0n666 an0n666 changed the title qBittorrent not checking if destination file exits when using Skip Hash Check. qBittorrent not checking if destination file exists when using Skip Hash Check. Jan 8, 2020
@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 13, 2020

@sledgehammer999 @glassez @Chocobo1
Please take a look into this issue if you get time. One could effectively fake a seeding state and get seed hours/ seedbonus in private trackers by taking advantage of it.

@FranciscoPombal

This comment has been minimized.

Copy link
Contributor

@FranciscoPombal FranciscoPombal commented Jan 13, 2020

Well, this part seems correct:

When a leecher connects and read piece is called, libtorrent knows file doesn’t exists so it starts downloading it again.

The problem is going directly into seeding state in the first place.

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 13, 2020

Well, can someone to summarize the problem?

  1. What's the typical use case of "Skip Hash Check" option?
  2. What can cause the problem?
  3. How "Skip Hash Check" should behave?
@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 13, 2020

  1. Case# 1. I’ve already downloaded a torrent from one tracker and want to seed that in another. I don’t want qBit to recheck the hash when adding the torrent from other tracker. Remember, info hashes can be different across trackers so trackers won’t merge in all use cases. A new torrent will get added and recheck will commence unless skip hash chk is used.
    Case#2. I’ve removed a torrent from qBit, want to re-seed it. Don’t want qbit to recheck.
    Case #3. I’m uploading a torrent to a private tracker. After uploading I’ve to redownload the metafile to add passkey. I won’t like to waste time in qBit rechecking the file again.

  2. Tick skip hash check when adding torrent.

  3. When that option is checked during adding torrent, check if the files already exist in the directory or not. If not present, don’t apply skip hash check, instead add that torrent as usual Without skipping hash checking.

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 13, 2020

Remember, info hashes can be different across trackers so trackers won’t merge in all use cases.

IIRC, we are talking about checking of files. Info hash has nothing to do here.

Tick skip hash check when adding torrent.

Do you mean the case when the user did this accidentally?

If not present, don’t apply skip hash check, instead add that torrent as usual Without skipping hash checking.

What's about the case when user doesn't really want to download anything? E.g. it has the torrent content previously downloaded (but files were renamed that time).

@FranciscoPombal

This comment has been minimized.

Copy link
Contributor

@FranciscoPombal FranciscoPombal commented Jan 13, 2020

@an0n666 even with the changes you are suggesting, a cheater could just create the correct folder structure + 0-size files with the correct names, and qBittorrent would go directly into seeding mode when "skip hash check" is on.

Perhaps it is better to get rid of the option altogether? It would be kind of inconvenient, especially for case 3.

But the metafile redownload in step 3 can generally be avoided as far as I know by setting the correct passkey and source flag (-s flag in mktorrent) from the start.

As for the other 2 cases, you'd just have to live with the recheck, which means a little extra "downtime".

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 13, 2020

Maybe it's better to go into "Missing files" state if option is enabled but there are no files exist?

@FranciscoPombal

This comment has been minimized.

Copy link
Contributor

@FranciscoPombal FranciscoPombal commented Jan 13, 2020

@glassez

Maybe it's better to go into "Missing files" state if option is enabled but there are no files exist?

Would not stop the potential cheating issue raised in the OPthe first comment, see my comment above.

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 13, 2020

Would not stop the potential cheating issue raised in the OP, see my comment above.

Sorry, I answered another comment.
BTW, I don't intend to fight with cheeters. F there is valid use case of this option I would fix it, otherwise let's remove it totally.

@FranciscoPombal

This comment has been minimized.

Copy link
Contributor

@FranciscoPombal FranciscoPombal commented Jan 13, 2020

Sorry, I answered another comment.
BTW, I don't intend to fight with cheeters. F there is valid use case of this option I would fix it, otherwise let's remove it totally.

If you ignore the cheating issue, all 3 cases mentioned above are legitimate uses of the feature. However, there should be a simple check for missing files and, like you mentioned:

Maybe it's better to go into "Missing files" state if option is enabled but there are no files exist?

Yep.

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 13, 2020

If you ignore the cheating issue, all 3 cases mentioned above are legitimate uses of the feature. However, there should be a simple check for missing files and, like you mentioned

Well, then I want to hear more opinions about "cheating issue". Should we drop "Skip hash check" option or not?
@sledgehammer999, @Chocobo1, @thalieht?

@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 13, 2020

@an0n666 even with the changes you are suggesting, a cheater could just create the correct folder structure + 0-size files with the correct names, and qBittorrent would go directly into seeding mode when "skip hash check" is on.

I think we should also call a random piece read so even if cheater has the correct directory and filename or filesize setup, if a randomly selected piece does not pass hash check then libtorrent will recheck that torrent anyway.

But the metafile redownload in step 3 can generally be avoided as far as I know by setting the correct passkey and source flag (-s flag in mktorrent) from the start.

Some trackers add their own unique key to info hash so unless the metafile is redownloaded you cannot seed.
@FranciscoPombal

IIRC, we are talking about checking of files. Info hash has nothing to do here.

By different hash i meant if info hash is different then qBit treats them as different torrents. So the use case of skip hash checking applies when adding same torrents with different info hash

Do you mean the case when the user did this accidentally?

If they tick it knowingly but select a wrong directory.

What's about the case when user doesn't really want to download anything?>
E.g. it has the torrent content previously downloaded (but files were renamed that time).

Missing files thing would solve the issue.
@glassez

@FranciscoPombal

This comment has been minimized.

Copy link
Contributor

@FranciscoPombal FranciscoPombal commented Jan 13, 2020

Well, then I want to hear more opinions about "cheating issue".

Just to be clear, I am not claiming it is prevalent. In fact, I hadn't thought about it until now.
A really determined cheater will always find a way cheat. But this seems to make it too easy.

@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 18, 2020

Well, then I want to hear more opinions about "cheating issue".

Just to be clear, I am not claiming it is prevalent. In fact, I hadn't thought about it until now.
A really determined cheater will always find a way cheat. But this seems to make it too easy.

I think it doesn't matter if this issue is prevalent or not. A client by default should not have any feature which could be exploited so easily by a potential cheater. Like anyone could add a thousand torrents with skip hash check and seed those with zero effort and gain seedtime/seedbonus.
I understand that this issue is irrevalent for public tracker users but it's pretty critical for private trackers. This kind of exploitation could warrant a ban of Qbit from private trackers as well unless addressed properly.

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 18, 2020

So, should we just drop it out?

@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 19, 2020

So, should we just drop it out?

If the feature can't be made more difficult to be exploited then it's better to drop it I guess.

@glassez

This comment has been minimized.

Copy link
Member

@glassez glassez commented Jan 19, 2020

If the feature can't be made more difficult to be exploited

It can more or less. But if it will be as expensive as a checking, it does not make sense.

@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 19, 2020

If the feature can't be made more difficult to be exploited

It can more or less. But if it will be as expensive as a checking, it does not make sense.

I think we have a less expensive way of doing it. Currently If a peer connects and libtorrent calls read piece and fails, that torrent resets to 0%.
If a torrent is added using skip hash, we could call read piece on a randomly selected piece which would emulate the same situation as a leecher connecting and requesting a piece from the seeder. Since libtorrent will fail to read the piece it’ll reset back to 0%. We’ll be reading just one random piece so this will be the least expensive way.

@an0n666

This comment has been minimized.

Copy link
Contributor Author

@an0n666 an0n666 commented Jan 19, 2020

I'm talking about this http://libtorrent.org/reference-Core.html#read_piece()
to be precise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.