5 ms·

Noteworthy that this wasn't a bug, but a "feature": void downloadThread::ignoreSslErrors(QNetworkReply* reply,QList<QSslError> errors) { // Ignore all SS

Noteworthy that this wasn't a bug, but a "feature": void downloadThread::ignoreSslErrors(QNetworkReply* reply,QList<QSslError> errors) { // Ignore all SSL errors reply->ignoreSslErrors(errors); } https://github.com/qbittorrent/qBittorrent/commit/9824d86a3cbecc9fd91f0462956ca575ef742110#diff-76a082d0ec1a72c138ddf25cb4da8146a925d25bedf993dedf57bb044e5ac19cR219 https://github.com/qbittorrent/qBittorrent/commit/9824d86a3c...

perching_aix 2y ago
Is the motivation behind this known?
- SushiHippie 2y ago
  As the commit message was "Fix HTTPS protocol support in torrent/rss downloader" I suppose it was a quick fix to make things work, and as things worked no one ever took a look at it until now. EDIT: The author of the PR[0] (who is one of the top qBittorrent contributors according to GitHub[1]) that fixed this also came to this conclusion: > I presume that it was a quick'n'dirty way to get SSL going which persisted to this day. It's also possible that back in the day Qt4 (?) didn't support autoloading ca root certificates from the OS's store. [0]: https://github.com/qbittorrent/qBittorrent/pull/21364 https://github.com/qbittorrent/qBittorrent/pull/21364 [1]: https://github.com/qbittorrent/qBittorrent/graphs/contributors https://github.com/qbittorrent/qBittorrent/graphs/contributo...
  - 0xsee4 2y ago
    To be fair, this function ignoreSslErrors is not from the authors of qBittorrent, it comes from QT framework. The idea behind the function is that you provide it a small whitelist of errors you wish to ignore, for example in a Dev build you may well want to ignore self-signed errors for your Dev environment. The trouble is, you can call it with no arguments and this means you will ignore every error. This may have been misunderstood by the qBittorrent maintainers, maybe not. Much more likely is that someone knew they had implemented this temporary solution while they implemented OpenSSL in a project which previously never had SSL support - a major change with a lot of work involved - and every programmer knows that there is nothing more permanent than a temporary solution. Especially in this case. I can understand how such code would make it into the repo(I think you do too), and it's very easy for us to say we would then have immediately amended it in the next version to properly verify certs. Having been in contact with the maintainers, I have to say I was disappointed in how seriously they took the issue. I don't want to say any more than that. Source: author of the article
    - bdelay 2y ago
      How much notification did you give the developers before you disclosed? Did you enforce a timeline?
      - metadat 2y ago
        Warning shots across the bow in private are the polite and responsible way, but malicious actors don't typically extend such courtesies to their victims. As such, compared to the alternative (bad actors having even more time to leverage and amplify the information asymmetry), a timely public disclosure is preferable, even with some unfortunate and unavoidable fallout. Typically security researchers are reasonable and want to do the right thing with regard to responsible disclosure. On average, the "bigger party" inherently has more resources to respond compared to the reporter. This remains true even in open source software.
        tsimionescu 2y ago
        This is a pretty dangerous take. The reality is that the vast majority of security vulnerabilities in software are not actively exploited, beause no one knows about them. Unless you have proof of active exploitation, you are much more likely to hurt users by publicly disclosing a 0-day than by responsibly disclosing it to the developer and giving them a reasonable amount of time to come out with a patch. Even if the developers are acting badly. Making a vulnerability public is putting a target on every user, not on the developer.
        wisemang 2y ago
        Your take is the dangerous one. I don’t disagree that > the vast majority of security vulnerabilities in software are not actively exploited However I’d say your explanation that it’s > because no one knows about them is not necessarily the reason why. If the vendor or developer isn’t fixing things, going public is the correct option. (I agree some lead time / attempt at coordinated disclosure is preferable here.)
  - SV_BubbleTime 2y ago
    Another point against the “security” of open source software. “Oh, it’ll have millions of eyes on it”… except no one looks.
    - Nadya 2y ago
      As opposed to the “security” of closed source software? Where severe vulns are left in as long as they aren't publicized because it would take too much development time to justify fixing and the company doesn't make money fixing vulns - it makes money creating new features. And since it isn't a security-related product any lapses in security are an "Oopsy woopsy we screwed up" and everyone moves on with their lives? Even companies that are supposed to get security right have constant screw ups that are only fixed when someone goes poking around where they probably shouldn't and thankfully happens to not be malicious.
      - LittleShaman 2y ago
        I think your comment works as a reply to claiming closed source is more secure than open source - you try to bring them both to the same level. I dont think it replies to what the user asks though. It seems reasonable expecting widely used open source software to be studied by many people. If thats true it would be good to question why this wasnt caught by anyone. Ignoring all ssl errors is not something you need to be an expert to know is bad...
        Nadya 2y ago
        Codebases outside of security-contexts are rarely audited, much less professionally so. The culture of code reviewing PR's from 14 years ago is a little different from today and is also why any "quick hacks to make things work" should always have some form of "//HACK: REVIEW OR REMOVE BY <DATE>" attached to it to make it easy to find. From a security perspective there are only two kinds of code bases: open & closed. By deduction one of those will have more eyeballs on the codebase than the other even if "nobody looks". Case in point: It may have taken 14 years but someone looked. Had the code base been closed source that may never have happened because it might not have been possible to ever happen. It's also very easy to point to the number of security issues that never made it into production because it was caught in an open source code review by passerbys and other contributors while the PR was waiting to be merged. The fact it was caught at all is a point for open source security - not against it. Even if it took 14 years.
- beeboobaa3 2y ago
  A guess that's probably correct: Many torrent sites (where the client can download .torrent files from when given an URL) their infra sucks. This includes expired certificates. Users don't want to deal with that shit. Developers don't want to deal with users complaining. It's not really considered a risk because lots of those torrent sites (used to) just use HTTP to begin with, so who cares, right?

perching_aix 2y ago

Is the motivation behind this known?

SushiHippie 2y ago
As the commit message was "Fix HTTPS protocol support in torrent/rss downloader" I suppose it was a quick fix to make things work, and as things worked no one ever took a look at it until now. EDIT: The author of the PR[0] (who is one of the top qBittorrent contributors according to GitHub[1]) that fixed this also came to this conclusion: > I presume that it was a quick'n'dirty way to get SSL going which persisted to this day. It's also possible that back in the day Qt4 (?) didn't support autoloading ca root certificates from the OS's store. [0]: https://github.com/qbittorrent/qBittorrent/pull/21364 https://github.com/qbittorrent/qBittorrent/pull/21364 [1]: https://github.com/qbittorrent/qBittorrent/graphs/contributors https://github.com/qbittorrent/qBittorrent/graphs/contributo...
- 0xsee4 2y ago
  To be fair, this function ignoreSslErrors is not from the authors of qBittorrent, it comes from QT framework. The idea behind the function is that you provide it a small whitelist of errors you wish to ignore, for example in a Dev build you may well want to ignore self-signed errors for your Dev environment. The trouble is, you can call it with no arguments and this means you will ignore every error. This may have been misunderstood by the qBittorrent maintainers, maybe not. Much more likely is that someone knew they had implemented this temporary solution while they implemented OpenSSL in a project which previously never had SSL support - a major change with a lot of work involved - and every programmer knows that there is nothing more permanent than a temporary solution. Especially in this case. I can understand how such code would make it into the repo(I think you do too), and it's very easy for us to say we would then have immediately amended it in the next version to properly verify certs. Having been in contact with the maintainers, I have to say I was disappointed in how seriously they took the issue. I don't want to say any more than that. Source: author of the article
  - bdelay 2y ago
    How much notification did you give the developers before you disclosed? Did you enforce a timeline?
    - metadat 2y ago
      Warning shots across the bow in private are the polite and responsible way, but malicious actors don't typically extend such courtesies to their victims. As such, compared to the alternative (bad actors having even more time to leverage and amplify the information asymmetry), a timely public disclosure is preferable, even with some unfortunate and unavoidable fallout. Typically security researchers are reasonable and want to do the right thing with regard to responsible disclosure. On average, the "bigger party" inherently has more resources to respond compared to the reporter. This remains true even in open source software.
      - tsimionescu 2y ago
        This is a pretty dangerous take. The reality is that the vast majority of security vulnerabilities in software are not actively exploited, beause no one knows about them. Unless you have proof of active exploitation, you are much more likely to hurt users by publicly disclosing a 0-day than by responsibly disclosing it to the developer and giving them a reasonable amount of time to come out with a patch. Even if the developers are acting badly. Making a vulnerability public is putting a target on every user, not on the developer.
        wisemang 2y ago
        Your take is the dangerous one. I don’t disagree that > the vast majority of security vulnerabilities in software are not actively exploited However I’d say your explanation that it’s > because no one knows about them is not necessarily the reason why. If the vendor or developer isn’t fixing things, going public is the correct option. (I agree some lead time / attempt at coordinated disclosure is preferable here.)
- SV_BubbleTime 2y ago
  Another point against the “security” of open source software. “Oh, it’ll have millions of eyes on it”… except no one looks.
  - Nadya 2y ago
    As opposed to the “security” of closed source software? Where severe vulns are left in as long as they aren't publicized because it would take too much development time to justify fixing and the company doesn't make money fixing vulns - it makes money creating new features. And since it isn't a security-related product any lapses in security are an "Oopsy woopsy we screwed up" and everyone moves on with their lives? Even companies that are supposed to get security right have constant screw ups that are only fixed when someone goes poking around where they probably shouldn't and thankfully happens to not be malicious.
    - LittleShaman 2y ago
      I think your comment works as a reply to claiming closed source is more secure than open source - you try to bring them both to the same level. I dont think it replies to what the user asks though. It seems reasonable expecting widely used open source software to be studied by many people. If thats true it would be good to question why this wasnt caught by anyone. Ignoring all ssl errors is not something you need to be an expert to know is bad...
      - Nadya 2y ago
        Codebases outside of security-contexts are rarely audited, much less professionally so. The culture of code reviewing PR's from 14 years ago is a little different from today and is also why any "quick hacks to make things work" should always have some form of "//HACK: REVIEW OR REMOVE BY <DATE>" attached to it to make it easy to find. From a security perspective there are only two kinds of code bases: open & closed. By deduction one of those will have more eyeballs on the codebase than the other even if "nobody looks". Case in point: It may have taken 14 years but someone looked. Had the code base been closed source that may never have happened because it might not have been possible to ever happen. It's also very easy to point to the number of security issues that never made it into production because it was caught in an open source code review by passerbys and other contributors while the PR was waiting to be merged. The fact it was caught at all is a point for open source security - not against it. Even if it took 14 years.
beeboobaa3 2y ago
A guess that's probably correct: Many torrent sites (where the client can download .torrent files from when given an URL) their infra sucks. This includes expired certificates. Users don't want to deal with that shit. Developers don't want to deal with users complaining. It's not really considered a risk because lots of those torrent sites (used to) just use HTTP to begin with, so who cares, right?