search

NASA-PDS / validate

Validates PDS4 product labels, data and PDS3 Volumes
https://nasa-pds.github.io/validate/
Apache License 2.0
16 stars 11 forks source link

Fix code scanning alerts #468

Open jordanpadams opened 2 years ago

jordanpadams commented 2 years ago

Tracking issue for:

These issue are unlikely to occur, but in the event of some sort of man-in-the-middle attack, we should be extra careful here.

Note: Per comments below, this ticket is waiting on the baseline schemas/schematrons to be registered so we can use the checksums to verify we are downloading the correct files.

jordanpadams commented 2 years ago

changed this bug severity to medium since this is a concern, but only really applies to server-side applications

al-niessner commented 1 year ago

@jordanpadams

Sorry, I am having a difficult time understanding the concern over these messages. Validate is not a service. It is a program run by a user and the same user that supplies the XML and schematron being ingested.

  1. Since it is not a service, what denial of service could possible be done? The user prevents themselves from running the tool? Just do not run it.
  2. If the user uses it to access files on their computer, then how is that different than ls and cat? I could see an issue if the validate was running as root or needed to run as root allowing a user to elevate their access, but this is not the case. The tool runs as the user and grants no extra access than the computer they are user gives them in the first place.
jordanpadams commented 1 year ago

@al-niessner I only glanced at these, but I was concerned in the event one of the schemas/schematrons being referenced by the labels was hacked. If these are all referring to the label input by the user, themselves, then I am OK with closing them out. Just wanted to have another set of eyes on it.

al-niessner commented 1 year ago

@jordanpadams

I just did not know if something changed like we were offering validate as a service now, which I think is a really bad idea.

Ideally, it would be great to tie something like the schematron to a checksum to verify the one downloaded is the expected. However, doing both from the same site is kind of useless and even doing it the same way (both downloads) weakens the check. Those are best with eyes to confirm downloaded checksum kind of things. Two wholly separate paths for the different information is best of course. Then having your own mods makes a mess of it all too. Still if PDS were/are amenable to having two independent sites, one with checksum and other with data, we could do a cursory check on those before proceeding but if the hack was to point away from PDS then it would still go undetected since users can do that for their own mods.

I would say that given the latitude for customization and extension to the user by PDS, doing security checks is not reasonable. We just have to trust the users are not self harming.

One the flip of that, might want to offer PDS a service where we scan their schema/schematron periodically and let them know when/if the checksum of one or more of them has changed where we keep the golden list independent of the PDS site. Detect changes at the site as well as redirect hacks. Should give the best bang for the buck. Maybe they do it already.

jordanpadams commented 12 months ago

thanks @al-niessner

One the flip of that, might want to offer PDS a service where we scan their schema/schematron periodically and let them know when/if the checksum of one or more of them has changed where we keep the golden list independent of the PDS site.

This is a great idea. We store the checksums in a DB somewhere and go back through and validate this.

Now that I think about it, if we just register these schemas (which we should), we then will those queryable from OpenSearch. Will add this to the icebox for now, but we can revisit once we actually register these schemas.

jordanpadams commented 12 months ago

Note: Since the likelihood is very very low (1), impact/consequence high (5), I am recategorizing this as a medium severity (s.medium).

https://github.com/NASA-PDS/nasa-pds.github.io/wiki/Issue-Tracking#bug-prioritization-policy