Switch FileSet's temporary file names over to using UUIDs about pydicom HOT 5 CLOSED

Hm, how are SOP Instance UIDs in practice different from UUIDs? I may not understand the problem... do you think somebody will write a file path in the UID so the file will be saved in a sensible location, or something like that?

from pydicom.

Keep in mind I have very little knowledge of this, but you can do:

1. Set the SOP Instance UID maliciously (ds.SOPInstanceUID = "/home/user/test.dcm")
2. ???
3. Profit - in my case, when I call FileSet.add(ds) I get the test.dcm file escaping the temporary directory and writing itself to my home directory.

When pydicom is setting the filenames using a random value, this is mitigated.

from pydicom.

from pydicom import dcmread
from pydicom.data import get_testdata_file
from pydicom.fileset import FileSet

fs = FileSet()
ds = dcmread(get_testdata_file("CT_small.dcm"))
ds.SOPInstanceUID = "/home/[user]/freeeeee.dcm"
fs.add(ds)

If you had a root level script running this could potentially be bad.

from pydicom.

Yeah ok, I guessed that you meant something like that. I'm also not an expert in security related stuff, so...
I think you are right and it makes sense to change this.

from pydicom.

I agree about it being a potential security issue, and am fine with the proposal, but thought of a possible alternative: what about just confirming UID is in fact a valid UID? Or more generally, confirm it has only digits and dots (and perhaps no ".." just to be extra safe).

However, this proposal does guarantee no conflicting values, and no security risk at all, so if it can work without too much added complexity, I'm good with that.

from pydicom.