security issue
Opened this issue · 10 comments
The following security issue just got published: GHSA-4wm9-3qmv-gvxj
This seems to have been reported actually already month ago. jsonicjs/jsonic#31
@rjrodger Do you need help maintaining these things?
There seems to be an uptick in these pretty spurious reports recently. This report is against internal undocumented utility functions.
to wit: https://gist.github.com/mestrtee
@mestrtee please stop annoying open source maintainers with this nonsense.
It recently hit our security pipelines and stopped production for us. I don't blame github for accepting, since the repro does indeed work.
My guess is, this is a bot, b/c it is only searching for prototype pollution. Which is quite easy to find with the right tools and kinda automatable.
@wzrdtales sorry to hear that - I reckon you'll keep getting hit by this at random for any deps you have - it is driving a lot of people crazy
What workaround did you use?
Easy quick fix was https://pnpm.io/package_json#pnpmauditconfigignorecves . pnpm is just in every regard the better npm
But we have been working on replacing seneca, which we in light of this event pushed through the door.
Can't fault you for that @wzrdtales. Thanks for the support over the years and I hope it was some use to you to get you to this point!
PS. I am doing a retrospective talk on seneca soon - do you have any notes or advice?
It was of tremendous value yes. Our reasons are strategical, financial and technical actually. We already hard-forked seneca some time ago, but the code was not maintainable for us, so it was rather sticking with that status quo than taking care of it. We went for efficiency and simplicity, to a) save quite some money (the ms landscape we rolled it out to yesterday went done from 2000 nodes to 860), roughly ~60% (we can push about 105k rps per node process without additional logic now, this made a bigger impact than we actually expected to see) and at least a ~40% improvement on general latencies across the board, and b) keep the code maintainable.
Notes:
- The pattern matching is very expensive, especially on larger objects. It is useful though, and we continue to use it, but a stripped down version (we have actually two versions, both are faster, but the stricter actE is exactly fitting our kind of use). In the end we never needed the full flexibility of it, but had always a static address first and eventually one or two parts of the data object that would come into play during dynamic endpoint selection.
- jsonic is slow, we replaced it with a simpler regex based implementation. only in repl we found jsonic in its full extend to be actually useful. otherwise its a very hard tradeoff of a terrible performance hit, vs the actual outcome of comfort which anyway in code never really gets used
- Transient history is not needed, the rare case where a loadbalancing will reiterate over a failed node because all other nodes in row failed as well and the original came back ofter the circuit breaking, almost never happens. So either keep it small at an LRU with TTL size of like 200-500, or embrace the system design of: absolutely everything is going to fail. We went for the latter, as the likelihood is so low, we didn't watch a single incident yet where this happened and we anyways need failure resistant systems
Advice:
Only one. Don't design your services too tiny, at least if latency is something of need, rather go for "macro" services, than microservices. Probably don't need to tell you that though.
There is quite some code that lives in the heart of the new backbone, we did not consider writing everything on our own completely, so we still use parts of seneca and still operate in the same way and with the same interfaces. We really just made this a drop in ( for our use case ).
Wow - that's really really useful - thank you for these notes! A lot to think about - in particular the balance of features vs performance.
Best of luck @wzrdtales !!!
Thanks @rjrodger will definitely still follow around.
And Github just removed it: github/advisory-database#4603 (comment) I took the freedom to report this false accusation of criticality.