Json commenting can introduce security issues




















In [] Fixes Applying patch from Kris Zyp to deprecate json-comment-filtered. Updated 1. This fix makes the use of json-comment-optional very verbose, even in the case where the server does not send commented json. This is also very confusing as the data is not commented, but the log shows warnings that commented json should not be used. Note also the implementation of json-comment-optional is not optimal, as non commented json will also ways try json-comment-filtered and fail with an Exception.

Forgot to account for json-comment-optional. Thanks to gregwilkins for tracking down the issue. Powered by Trac 1. Opened 14 years ago Closed 14 years ago. Sorry, something went wrong. I do not think this should be 3rd party tooling, nor do I think it's ideal to have an additional translation step between changing your composer. This should be something solved in Composer itself, I think.

Please argue your point beyond "I like YAML better" so that we can actually accomplish something here. There are some considerable downsides to your solution or even adding a YAML to JSON converter directly in Composer , and not a lot of upsides especially when compared to the other three solutions. So, if you add converter, then it is must be transparency for other developers. Adding this step to workflow should be a conscious decision.

And that's why it has to be a third-party solution, not part of the Composer. Duplicate of , , , I hope that it will be useful for you. To respond to your specific points from before:. Nobody said it was, but it is a standard that we can point at to say "This is what is allowed in composer. We can just use one. I'm aware. I don't think you read the suggestion thoroughly enough, but to reiterate, we would strip the comments before passing the JSON along to ext-json.

There wouldn't be any parse errors in this case. You can bang this drum all you like, but at the end of the day, you still can't comment your composer.

I'm not sure why ext-json would need to be involved. Changing the JSON spec is not realistic. Changing ext-json to tolerate non-standard JSON is not realistic. Adding some very simple logic to Composer is realistic. The other three issues you linked to are specifically about switching to YAML and are therefore not relevant to this issue. If no, why not? Seldaek :. I already volunteered to do the work for those projects.

That is, only people that opt-in to actually using comments in their root package's composer. My guess is that most of the time, this functionality won't be used in a package that actually gets pushed to Packagist anyway, though.

It would be more useful for private, internal projects that are just pulling in dependencies from Packagist or whatever other repositories. Validate will complain maybe but we could perhaps fix that. Sure composer validate sees it as invalid but it's otherwise not an issue at all. Well, composer validate actually runs as part of my build process.

If composer validate says it's bad, the build stops. That's easily fixed, sure, but it's also a pretty verbose way of writing comments.

I also don't see the benefit of comments as justifying such a mess of having two files. Yeah, this is definitely not an ideal solution. I only suggested it for the sake of completeness. We all acknowledge that comments are good, but every time something has been suggested that attempts to move Composer in that direction, the issue has been closed and conversation stops.

That's where my frustration is coming from, so apologies if that came across poorly. Third-party tooling meaning any tool out there reading a composer. For instance, your IDE is probably reading it and it does not use the Composer codebase for that, unlike Packagist and Satis , but they are not the only ones.

Some parsers simply ignore them as they should. What if all 3 exist and I do composer update? What if 2 of them exist and have different dependencies? Would we also have a composer.

Would we need to have similar logic in installed. JSON injections are not very common and not as dangerous as many other vulnerabilities but they can lead to other dangerous attacks, such as cross-site scripting XSS.

Read more about cross-site scripting XSS. Read more about general best practices for secure coding. JSON Injection vs. The attacker creates a malicious website and embeds a script tag in its code that attempts to access JSON data from the attacked web application. How about just special casing this? A format without comments is not feasable for human-written configuration files imho. Or is there some automated way to convert to a comment-free package. The larger that package.

I would add that a lot of tools now support configuration from within package. Also npm scripts are getting a lot of momentum and it's frustrating to not be able to add explanations for my future self.

JSON5 seems a great solution for this. NPM could start supporting package. Moreover, it could be a good opportunity to change name to something more sensical like npm. They both allow comments. This is highly unlikely to ever change in npm itself. To change it within npm itself at this point would be a truly massive undertaking, especially when set against the gain.

As hints, having an executable manifest leads to whole classes of security flaws and the complexity of YAML has, in reality, caused problems for the Ruby world for this reason. As hints, and as many comments on this issue explicitly say, there's nothing preventing you from creating a different representation of the metadata in package. Because this is unlikely to change, further bikeshedding is unlikely to be productive.

Therefore, I'm locking this issue. Skip to content. This repository has been archived by the owner. It is now read-only. Star Support for comments in package. Linked pull requests. Copy link. Yeah, this is not happening. Can you also describe WHY it is not happening?

I'd love to use yapm, but it's a real fork of npm There is a simple wrapper, versions yapm 0. Any other suggestions? TypeError: Object. Notice my comments were at top level.



0コメント

  • 1000 / 1000