Most active commenters
  • codedokode(3)

←back to thread

277 points jwilk | 20 comments | | HN request time: 1.502s | source | bottom
Show context
arp242 ◴[] No.44382233[source]
A lot of these "security bugs" are not really "security bugs" in the first place. Denial of service is not resulting in people's bank accounts being emptied or nude selfies being spread all over the internet.

Things like "panics on certain content" like [1] or [2] are "security bugs" now. By that standard anything that fixes a potential panic is a "security bug". I've probably fixed hundreds if not thousands of "security bugs" in my career by that standard.

Barely qualifies as a "security bug" yet it's rated as "6.2 Moderate" and "7.5 HIGH". To say nothing of gazillion "high severity" "regular expression DoS" nonsense and whatnot.

And the worst part is all of this makes it so much harder to find actual high-severity issues. It's not harmless spam.

[1]: https://github.com/gomarkdown/markdown/security/advisories/G...

[2]: https://rustsec.org/advisories/RUSTSEC-2024-0373.html

replies(13): >>44382268 #>>44382299 #>>44382855 #>>44384066 #>>44384368 #>>44384421 #>>44384513 #>>44384791 #>>44385347 #>>44385556 #>>44389612 #>>44390124 #>>44390292 #
1. codedokode ◴[] No.44384513[source]
Dereferencing a null pointer is an error. It is a valid bug.

The maintainer claims this is caused by allocator failure (malloc returning null), but it is still a valid bug. If you don't want to deal with malloc failures, just crash when malloc() returns null, instead of not checking malloc() result at all.

The maintainer could just write a wrapper around malloc that crashes on failure and replace all calls with the wrapper. It seems like an easy fix. Because almost no software can run where there is no heap memory so it makes no sense for the program to continue.

Another solution is to propagate every error back to the caller, but it is difficult and there is high probability that the caller won't bother checking the result because of laziness.

A quote from a bug report [1]:

> If xmlSchemaNewValue returns NULL (e.g., due to a failure of malloc), xmlSchemaDupVal checks for this and returns NULL.

[1] https://gitlab.gnome.org/GNOME/libxml2/-/issues/905

replies(5): >>44384716 #>>44385168 #>>44385200 #>>44386885 #>>44387099 #
2. worthless-trash ◴[] No.44384716[source]
A while back i remember looking at the kernel source code, when overcommit is enabled, malloc would not fail if it couldnt allocate memory, it would ONLY fail if you attempted to allocate memory larger than the available memory space.

I not think you can deal with the failure condition the way you think on Linux (and I imagine other operating systems too).

replies(2): >>44385434 #>>44386490 #
3. fredilo ◴[] No.44385168[source]
> The maintainer could just write a wrapper around malloc that crashes on failure and replace all calls with the wrapper. It seems like an easy fix. Because almost no software can run where there is no heap memory so it makes no sense for the program to continue.

So could the reporter of the bug. Alternatively, he could add an `if(is null){crash}` after the malloc. The fix is easy for anyone that has some knowledge of the code base. The reporter has demonstrated this knowledge in finding the issue.

If a useful PR/patch diff was provided with the reporter, I would have expected it to be merged right away.

However, instead of doing the obvious thing to actually solve the issue, the reporter hits the maintainer with this bureaucratic monster:

> We'd like to inform you that we are preparing publications on the discovered vulnerability.

> Our Researchers plan to release the technical research, which will include the description and details of the discovered vulnerability.

> The research will be released after 90 days from the date you were informed of the vulnerability (approx. August 5th, 2025).

> Please answer the following questions:

>

> * When and in what version will you fix the vulnerability described in the Report? (date, version)

> * If it is not possible to release a patch in the next 90 days, then please indicate the expected release date of the patch (month).

> * Please, provide the CVE-ID for the vulnerability that we submitted to you.

>

> In case you have any further questions, please, contact us.

https://gitlab.gnome.org/GNOME/libxml2/-/issues/905#note_243...

The main issue here is really one of tone. The maintainer has been investing his free time to altruistically move the state of software forward and the reporter is too lazy to even type up a tone-adjusted individual message. Would it have been so hard for the reporter to write the following?

> Thank you for your nice library. It is very useful to us! However, we found a minor error that unfortunately might be severely exploitable. Attached is a patch that "fixes" it in an ad-hoc way. If you want to solve the issue in a different way, could we apply the patch first, and then you refactor the solution when you find time? Thanks! Could you give us some insights on when after merging to main/master, the patch will end up in a release? This is important for us to decide whether we need to work with a bleeding edge master version. Thank you again for your time!

Ultimately, it is a very similar message content. However, it feels completely different.

Suppose you are a maintainer without that much motivation left, and you get hit with such a message. You will feel like the reporter is an asshole. (I'm not saying he is one.) Do you really care, if he gets powned via this bug? It takes some character strength on the side of the maintainer to not just leave the issue open out of spite.

replies(3): >>44385549 #>>44386251 #>>44387283 #
4. saurik ◴[] No.44385200[source]
> It is a valid bug.

But is it a high-severity security bug?

replies(1): >>44385763 #
5. codedokode ◴[] No.44385434[source]
The bug was about the case when malloc returns null, but the library doesn't check for it.
replies(1): >>44385961 #
6. sersi ◴[] No.44385549[source]
> the reporter is too lazy to even type up a tone-adjusted individual message. Would it have been so hard for the reporter to write the following?

The reporter doesn't care about libxml2 being more secure, they only care about having a CVE-ID to brag about discovering a vulnerability and publishing it on their blog. If the reporter used the second message you wrote, they wouldn't get what they want.

7. Quekid5 ◴[] No.44385763[source]
Considering that it's Undefined Behavior, quite possibly.

EDIT: That said, I'm on the maintainer's side here.

replies(1): >>44386062 #
8. bjourne ◴[] No.44385961{3}[source]
Correct, but the point is that it is difficult to get malloc to return null on Linux. Why litter your code with checks for de facto impossible scenarios?
replies(2): >>44386485 #>>44387412 #
9. gpderetta ◴[] No.44386062{3}[source]
> Considering that it's Undefined Behavior, quite possibly.

Is it thought? Certainly it is according the C and C++ standards, but POSIX adds:

> References to unmapped addresses shall result in a SIGSEGV signal

While time-traveling-UB is a theoretical possibility, practically POSIX compliant compilers won't reorder around potentially trapping operations (they will do the reverse, they might remove a null check if made redundant by a prior potentially trapping dereference) .

A real concern is if a null pointer is dereferenced with a large attacker-controlled offset that can avoid the trap, but that's more of an issue of failing to bound check.

replies(1): >>44386336 #
10. rwmj ◴[] No.44386251[source]
If someone had reported that on a project I maintain, I'd have told them to get outta here, in somewhat less polite language. They're quite clearly promoting their own company / services and don't care in the slightest about libxml2.
replies(1): >>44387804 #
11. ynik ◴[] No.44386336{4}[source]
Under your interpretation, neither gcc nor clang are POSIX compliant. Because in practice all these optimizing compilers will reorder memory accesses without bothering to prove that the pointers involved are valid -- the compiler just assumes that the pointers are valid, which is justified because otherwise the program would have undefined behavior.
replies(2): >>44386394 #>>44386634 #
12. gpderetta ◴[] No.44386394{5}[source]
Actually you are right, what I said about reordering is nonsense. The compiler will definitely reorder non-aliasing accesses. There are much weaker properties that are preserved.
13. daef ◴[] No.44386485{4}[source]
in systems level programming (the introductory course before operating systems in our university) this was one of the first misconceptions to be eradicated. you cannot trust malloc to return null.
14. vbezhenar ◴[] No.44386490[source]
It's very easy to make malloc return NULL:

  % ulimit -v 80000
  
  % cat test.c
  #include <stdio.h>
  #include <stdlib.h>
  
  int main(void) {
    char *p = malloc(100'000'000);
    printf("%p\n", p);
  }
  
  % cc test.c
  
  % ./a.out
  (nil)
15. somat ◴[] No.44386634{5}[source]
I am not so sure. Assuming the program does not do anything undefined is sort of the worst possible take on leaving the behavior undefined in the first place. I mean the behavior was left undefined so that "something" could be done. the language standard just does not know what that "something" is. Hell the compiler could do nothing and that would make more sense.

But to make optimizations pretending it is an invariant, it can't happen, when the specification clearly says it could happen. That's wild, and I would argue out of specification.

16. andrewaylett ◴[] No.44386885[source]
Many systems have (whether you like the idea or not) effectively infallible allocators. If malloc won't ever return null, there's not much point in checking.
17. sidewndr46 ◴[] No.44387099[source]
in the event that malloc returns NULL and it isn't checked, isn't the program going to crash anyways? I usually just use a macro like "must_malloc" that does this anyways. But the out come is the same I would think. It's mostly a difference of where it happens.
18. yrro ◴[] No.44387283[source]
If I received an email like that I'd reply with an invoice.
19. codedokode ◴[] No.44387412{4}[source]
First, Linux has thousands of settings that could affect this, second the library probably works not only on Linux.
20. Pet_Ant ◴[] No.44387804{3}[source]
I mean, no security researchers do. It's very much like capitalists. They aren't trying to do something to improve society, but by persuing their own private incentives, they end up with behaviour that benefits the commons. Sometimes we need regulations around that in the marketplace, and that's what the FTC is. So we need an OSS-social-contract version of that.

It's kind of like the enshitification of bug reports. The best way to deal with it is probably denying them CVE numbers to disincentivise the look of low hanging fruit that reasonably could be done by a linter.

Reminds me of students juicing their PRs be making changes to typos in documentation and comments just to put it on their resumes.