ddp/photoassistant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

made SafePath and ensured initital paths and move_paths have valid paths, and all filenames have to be found via os.walk so should be impossible to write to parts of the FS that are unsafe

Browse Source
This commit is contained in:
Damien De Paoli
2023-02-03 16:57:39 +11:00
parent 4b1bbcb2bf
commit 2b478ed505
3 changed files with 78 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
TODO
View File

@@ -1,10 +1,4 @@
### GENERAL ### GENERAL
* think about security - in job_mgr anywhere I can os.replace/remove NEED to protect, etc
- just need to use this I think:
from werkzeug.utils import secure_filename
secure_filename(xxxx)
* change the rotation code to use that jpeg util to reduce/remove compression loss? * change the rotation code to use that jpeg util to reduce/remove compression loss?
* read this: https://flask.palletsprojects.com/en/2.2.x/testing/#faking-resources-and-context * read this: https://flask.palletsprojects.com/en/2.2.x/testing/#faking-resources-and-context
@@ -34,6 +28,8 @@
files.py:@app.route("/fix_dups", methods=["POST"]) files.py:@app.route("/fix_dups", methods=["POST"])
??? ???
* allow user to choose default log level to show
* GUI overhaul? * GUI overhaul?
* on a phone, the files.html page header is a mess "Oldest.." line is too large to fit on 1 line (make it a hamburger?) * on a phone, the files.html page header is a mess "Oldest.." line is too large to fit on 1 line (make it a hamburger?)
- searched for text overlaps buttons above and below - searched for text overlaps buttons above and below

98
pa_job_manager.py
View File

@@ -577,14 +577,18 @@ def MessageToFE( job_id, message, level, persistent, cant_close ):
def SettingsRBPath(): def SettingsRBPath():
settings = session.query(Settings).first() settings = session.query(Settings).first()
if settings == None: if settings == None:
print("Cannot create file data with no settings / recycle bin path is missing") print("ERROR: Cannot create file data with no settings / recycle bin path is missing")
return return None
# path setting is an absolute path, just use it, otherwise prepend base_path first # path setting is an absolute path, just use it, otherwise prepend base_path first
p=settings.recycle_bin_path if settings.recycle_bin_path[0] == '/':
if p == '/': path=settings.recycle_bin_path
return p
else: else:
return settings.base_path+p path=settings.base_path+settings.recycle_bin_path
if SafePath(path):
return path
else:
print ("ERROR: recycle bin path contains unsafe characters")
return None
############################################################################## ##############################################################################
@@ -593,7 +597,7 @@ def SettingsRBPath():
############################################################################## ##############################################################################
def ProcessRecycleBinDir(job): def ProcessRecycleBinDir(job):
path = SettingsRBPath() path = SettingsRBPath()
if not os.path.exists( path ): if path == None or not os.path.exists( path ):
AddLogForJob( job, f"Not Importing {path} -- Path does not exist" ) AddLogForJob( job, f"Not Importing {path} -- Path does not exist" )
return return
@@ -611,13 +615,17 @@ def ProcessRecycleBinDir(job):
def SettingsSPath(): def SettingsSPath():
settings = session.query(Settings).first() settings = session.query(Settings).first()
if settings == None or settings.storage_path == "": if settings == None or settings.storage_path == "":
print("Cannot create file data with no settings / storage path is missing") print("ERROR: Cannot create file data with no settings / storage path is missing")
return return None
p=settings.storage_path if settings.storage_path[0] == '/':
if p[0] == '/': path=settings.storage_path
return p
else: else:
return settings.base_path+p path=settings.base_path+settings.storage_path
if SafePath(path):
return path
else:
print ("ERROR: storage path contains unsafe characters")
return None
############################################################################## ##############################################################################
# ProcessStorageDirs(): wrapper func to call passed in job for each # ProcessStorageDirs(): wrapper func to call passed in job for each
@@ -636,15 +644,18 @@ def ProcessStorageDirs(parent_job):
def SettingsIPath(): def SettingsIPath():
paths=[] paths=[]
settings = session.query(Settings).first() settings = session.query(Settings).first()
if settings == None or settings.import_path == "": if not settings or settings.import_path == "":
print("Cannot create file data with no settings / import path is missing") print("ERROR: Cannot create file data with no settings / import path is missing")
return return None
p=settings.import_path if settings.import_path[0] == '/':
if p[0] == '/': path=settings.import_path
return p
else: else:
return settings.base_path+p path=settings.base_path+settings.import_path
if SafePath(path):
return path
else:
print ("ERROR: import path contains unsafe characters")
return None
############################################################################## ##############################################################################
# SettingsMPath(): return path to actual metadata path from settings # SettingsMPath(): return path to actual metadata path from settings
@@ -652,13 +663,29 @@ def SettingsIPath():
def SettingsMPath(): def SettingsMPath():
settings = session.query(Settings).first() settings = session.query(Settings).first()
if not settings or settings.metadata_path == "": if not settings or settings.metadata_path == "":
print ("WARNING: no Settings for metadata path") print ("ERROR: no Settings for metadata path")
return None return None
p=settings.metadata_path if settings.metadata_path[0] == '/':
if p[0] == '/': path=settings.metadata_path
return p
else: else:
return settings.base_path+p path=settings.base_path+settings.metadata_path
if SafePath(path):
return path
else:
print ("ERROR: metadata path contains unsafe characters")
return None
##############################################################################
# SafePath(): checks to see if a path is safe (for now basic) checks:
# no ..
##############################################################################
def SafePath(path):
if '..' in path:
return False
else:
return True
############################################################################## ##############################################################################
@@ -737,6 +764,7 @@ def CleanFileFromBin(job, e):
# use ctime as that will be when the file was moved into the Bin path # use ctime as that will be when the file was moved into the Bin path
if (now - stat.st_ctime)/SECS_IN_A_DAY >= settings.bin_cleanup_file_age: if (now - stat.st_ctime)/SECS_IN_A_DAY >= settings.bin_cleanup_file_age:
try: try:
# SAFE: as fname is constructed from entry on DB, had to exist on FS to be in the DB (no user-input)
os.remove( fname ) os.remove( fname )
except Exception as ex: except Exception as ex:
AddLogForJob(job, f"ERROR: Tried to delete old file: {ex}" ) AddLogForJob(job, f"ERROR: Tried to delete old file: {ex}" )
@@ -778,11 +806,13 @@ def JobMetadata(job):
if which == 'add_force_match_override' or which=='remove_force_match_override': if which == 'add_force_match_override' or which=='remove_force_match_override':
person_id=[jex.value for jex in job.extra if jex.name == "person_id"][0] person_id=[jex.value for jex in job.extra if jex.name == "person_id"][0]
p=session.query(Person).get(person_id) p=session.query(Person).get(person_id)
# SAFE: as dir is constructed from SafePath(Mpath) + data I control, no direct user-input
os.makedirs( f"{SettingsMPath()}force_match_overrides", mode=0o777, exist_ok=True ) os.makedirs( f"{SettingsMPath()}force_match_overrides", mode=0o777, exist_ok=True )
fname=f"{SettingsMPath()}force_match_overrides/{face_id}_{p.tag}" fname=f"{SettingsMPath()}force_match_overrides/{face_id}_{p.tag}"
elif which == 'add_no_match_override' or which == 'remove_no_match_override': elif which == 'add_no_match_override' or which == 'remove_no_match_override':
type_id=[jex.value for jex in job.extra if jex.name == "type_id"][0] type_id=[jex.value for jex in job.extra if jex.name == "type_id"][0]
t=session.query(FaceOverrideType).get(type_id) t=session.query(FaceOverrideType).get(type_id)
# SAFE: as dir is constructed from SafePath(Mpath) + data I control, no direct user-input
os.makedirs( f"{SettingsMPath()}no_match_overrides", mode=0o777, exist_ok=True ) os.makedirs( f"{SettingsMPath()}no_match_overrides", mode=0o777, exist_ok=True )
fname=f"{SettingsMPath()}no_match_overrides/{face_id}_{t.name}" fname=f"{SettingsMPath()}no_match_overrides/{face_id}_{t.name}"
else: else:
@@ -793,6 +823,7 @@ def JobMetadata(job):
file_h.write(f.face) file_h.write(f.face)
file_h.close() file_h.close()
else: else:
# SAFE: as fname is constructed from SafePath(Mpath) + data I control, no direct user-input
os.remove( fname ) os.remove( fname )
except Exception as ex: except Exception as ex:
AddLogForJob(job, f"ERROR: Error with metadata file '{fname}': {ex}" ) AddLogForJob(job, f"ERROR: Error with metadata file '{fname}': {ex}" )
@@ -1003,6 +1034,7 @@ def DisconnectSingleNoMatchOverride( job, o ):
new_fname=f'{mpath}0_{ot.name}_{md5face(f.face)}' new_fname=f'{mpath}0_{ot.name}_{md5face(f.face)}'
try: try:
if os.path.exists( fname ): if os.path.exists( fname ):
# SAFE: as SafePaths(mpath) combined with data I control in this func
os.replace( fname, new_fname ) os.replace( fname, new_fname )
else: else:
file_h=open( new_fname, 'wb') file_h=open( new_fname, 'wb')
@@ -1038,6 +1070,7 @@ def DisconnectSingleForceMatchOverride( job, o ):
new_fname=f'{path}0_{p.tag}_{md5face(f.face)}' new_fname=f'{path}0_{p.tag}_{md5face(f.face)}'
try: try:
if os.path.exists( fname ): if os.path.exists( fname ):
# SAFE: as SafePaths(mpath) combined with data I control in this func
os.replace( fname, new_fname ) os.replace( fname, new_fname )
else: else:
file_h=open( new_fname, 'wb') file_h=open( new_fname, 'wb')
@@ -1103,6 +1136,7 @@ def CreateSymlink(job,ptype,path):
if not os.path.exists(symlink): if not os.path.exists(symlink):
print( f"INFO: symlink does not exist, actually creating it -- s={symlink}" ) print( f"INFO: symlink does not exist, actually creating it -- s={symlink}" )
try: try:
# SAFE: SafePath() on init forces symlink to be safe
os.makedirs( os.path.dirname(symlink), mode=0o777, exist_ok=True ) os.makedirs( os.path.dirname(symlink), mode=0o777, exist_ok=True )
os.symlink(path, symlink) os.symlink(path, symlink)
except Exception as e: except Exception as e:
@@ -1268,9 +1302,11 @@ def RestoreFile(job,restore_me):
try: try:
# rel_path for a file in the Bin, is like 'Import/images_to_process/1111', so just prepend static/ # rel_path for a file in the Bin, is like 'Import/images_to_process/1111', so just prepend static/
dst_dir='static/' + restore_me.in_dir.rel_path + '/' dst_dir='static/' + restore_me.in_dir.rel_path + '/'
# SAFE: as SafePaths(rbpath) combined with data I control in this func (explicit 'static/' + DB entry path)
os.makedirs( dst_dir,mode=0o777, exist_ok=True ) os.makedirs( dst_dir,mode=0o777, exist_ok=True )
src=restore_me.FullPathOnFS() src=restore_me.FullPathOnFS()
dst=dst_dir + '/' + restore_me.name dst=dst_dir + '/' + restore_me.name
# SAFE: as SafePaths(rbpath) combined with data I control in this func (explicit 'static/' + DB entry path)
os.replace( src, dst ) os.replace( src, dst )
except Exception as e: except Exception as e:
AddLogForJob( job, f"ERROR: Failed to restores (mv) file on filesystem - which={src} to {dst}, err: {e}") AddLogForJob( job, f"ERROR: Failed to restores (mv) file on filesystem - which={src} to {dst}, err: {e}")
@@ -1321,6 +1357,7 @@ def MoveFileToRecycleBin(job,del_me):
os.makedirs( dst_dir,mode=0o777, exist_ok=True ) os.makedirs( dst_dir,mode=0o777, exist_ok=True )
src=del_me.FullPathOnFS() src=del_me.FullPathOnFS()
dst=dst_dir + '/' + del_me.name dst=dst_dir + '/' + del_me.name
# SAFE: as SafePaths(rbpath) combined with data I control in this func (explicit remove of 'static/' + DB entry path)
os.replace( src, dst ) os.replace( src, dst )
if DEBUG: if DEBUG:
print( f"MoveFileToRecycleBin({job.id},{del_me.name}): os.replace {src} with {dst} " ) print( f"MoveFileToRecycleBin({job.id},{del_me.name}): os.replace {src} with {dst} " )
@@ -1397,6 +1434,7 @@ def MoveEntriesToOtherFolder(job, move_me, dst_storage_path, dst_rel_path):
ResetAnySubdirPaths( move_me, dst_storage_path, move_me.dir_details.rel_path ) ResetAnySubdirPaths( move_me, dst_storage_path, move_me.dir_details.rel_path )
# move the actual dir to its new location # move the actual dir to its new location
try: try:
# SAFE: paths are from DB (entries already existed on FS, no user-input)
os.replace( orig_fs_pos, move_me.FullPathOnFS() ) os.replace( orig_fs_pos, move_me.FullPathOnFS() )
except Exception as e: except Exception as e:
AddLogForJob( job, f"ERROR: Failed to move dir: {orig_fs_pos} into {move_me.FullPathOnFS()}, err: {e}") AddLogForJob( job, f"ERROR: Failed to move dir: {orig_fs_pos} into {move_me.FullPathOnFS()}, err: {e}")
@@ -1416,6 +1454,7 @@ def MoveEntriesToOtherFolder(job, move_me, dst_storage_path, dst_rel_path):
session.add(move_me) session.add(move_me)
ResetAnySubdirPaths( move_me, dst_storage_path, dst_rel_path ) ResetAnySubdirPaths( move_me, dst_storage_path, dst_rel_path )
try: try:
# SAFE: paths are from DB (entries already existed on FS, no user-input)
os.replace( orig_fs_pos, move_me.FullPathOnFS() ) os.replace( orig_fs_pos, move_me.FullPathOnFS() )
except Exception as e: except Exception as e:
AddLogForJob( job, f"ERROR: Failed to rename dir: {orig_fs_pos} -> {move_me.FullPathOnFS()}, err: {e}") AddLogForJob( job, f"ERROR: Failed to rename dir: {orig_fs_pos} -> {move_me.FullPathOnFS()}, err: {e}")
@@ -1441,6 +1480,7 @@ def MoveEntriesToOtherFolder(job, move_me, dst_storage_path, dst_rel_path):
# move the actual file to its new location # move the actual file to its new location
AddLogForJob( job, f"DEBUG: move of FILE - {orig_fs_pos} -> {move_me.FullPathOnFS()}" ) AddLogForJob( job, f"DEBUG: move of FILE - {orig_fs_pos} -> {move_me.FullPathOnFS()}" )
try: try:
# SAFE: paths are from DB (entries already existed on FS, no user-input)
os.replace( orig_fs_pos, move_me.FullPathOnFS() ) os.replace( orig_fs_pos, move_me.FullPathOnFS() )
except Exception as e: except Exception as e:
AddLogForJob( job, f"ERROR: Failed to move file: {orig_fs_pos} -> {move_me.FullPathOnFS()}, err: {e}") AddLogForJob( job, f"ERROR: Failed to move file: {orig_fs_pos} -> {move_me.FullPathOnFS()}, err: {e}")
@@ -2018,6 +2058,7 @@ def GenVideoThumbnail( job, fname):
.run(capture_stdout=True, capture_stderr=True) .run(capture_stdout=True, capture_stderr=True)
) )
thumbnail, w, h = GenThumb( tmp_fname, False ) thumbnail, w, h = GenThumb( tmp_fname, False )
# SAFE: as tmp_fname is constructed from data I control in this func and data from entry which had to exist on FS to be in the DB (no user-input)
os.remove( tmp_fname ) os.remove( tmp_fname )
except ffmpeg.Error as e: except ffmpeg.Error as e:
AddLogForJob( job, f"ERROR: Failed to Generate thumbnail for video file: {fname} - error={e}" ) AddLogForJob( job, f"ERROR: Failed to Generate thumbnail for video file: {fname} - error={e}" )
@@ -2135,6 +2176,7 @@ def JobMoveFiles(job):
JobProgressState( job, "In Progress" ) JobProgressState( job, "In Progress" )
prefix=[jex.value for jex in job.extra if jex.name == "prefix"][0] prefix=[jex.value for jex in job.extra if jex.name == "prefix"][0]
suffix=[jex.value for jex in job.extra if jex.name == "suffix"][0] suffix=[jex.value for jex in job.extra if jex.name == "suffix"][0]
# SAFE: this stops any user-input into the move call that could flow into future os.* calls
# Sanity check, if prefix starts with /, reject it -> no /etc/shadow potentials # Sanity check, if prefix starts with /, reject it -> no /etc/shadow potentials
# Sanity check, if .. in prefix or suffix, reject it -> no ../../etc/shadow potentials # Sanity check, if .. in prefix or suffix, reject it -> no ../../etc/shadow potentials
# Sanity check, if // in prefix or suffix, reject it -> not sure code wouldnt try to make empty dirs, and I dont want to chase /////// cases, any 2 in a row is enough to reject # Sanity check, if // in prefix or suffix, reject it -> not sure code wouldnt try to make empty dirs, and I dont want to chase /////// cases, any 2 in a row is enough to reject
@@ -2264,6 +2306,7 @@ def ReloadMetadata(job):
session.add( DisconnectedNoMatchOverride( face=face_data, type_id=otype.id ) ) session.add( DisconnectedNoMatchOverride( face=face_data, type_id=otype.id ) )
if face_id: if face_id:
try: try:
# SAFE: as SafePaths(mpath) combined with data I control in this func
os.replace( fname, f'{mpath}no_match_overrides/0_{otype.name}_{md5face(face_data)}' ) os.replace( fname, f'{mpath}no_match_overrides/0_{otype.name}_{md5face(face_data)}' )
except Exception as ex: except Exception as ex:
print( f"ERROR: renaming no-match metadata on filesystem failed: {ex}" ) print( f"ERROR: renaming no-match metadata on filesystem failed: {ex}" )
@@ -2292,6 +2335,7 @@ def ReloadMetadata(job):
# if face>0, then we need to move the FS copy to a disco # if face>0, then we need to move the FS copy to a disco
if face_id: if face_id:
try: try:
# SAFE: as SafePaths(mpath) combined with data I control in this func
os.replace( fname, f'{mpath}force_match_overrides/0_{p.tag}_{md5face(face_data)}' ) os.replace( fname, f'{mpath}force_match_overrides/0_{p.tag}_{md5face(face_data)}' )
except Exception as ex: except Exception as ex:
print( f"ERROR: renaming force-match metadata on filesystem failed: {ex}" ) print( f"ERROR: renaming force-match metadata on filesystem failed: {ex}" )
@@ -2430,6 +2474,7 @@ def AddFaceToFile( locn_data, face_data, file_eid, model_id, settings ):
# can only be 1 match with the * being a UUID # can only be 1 match with the * being a UUID
fname=glob.glob( f'{path}0_{p.tag}_*' )[0] fname=glob.glob( f'{path}0_{p.tag}_*' )[0]
new_fname=f'{path}{face.id}_{p.tag}' new_fname=f'{path}{face.id}_{p.tag}'
# SAFE: as SafePaths(mpath) combined with data I control in this func
os.replace( fname, new_fname ) os.replace( fname, new_fname )
except Exception as ex: except Exception as ex:
print( f"ERROR: AddFaceToFile-face connects to 'disconnected-force-match' metadata, but fixing the filesystem metadata failed: {ex}" ) print( f"ERROR: AddFaceToFile-face connects to 'disconnected-force-match' metadata, but fixing the filesystem metadata failed: {ex}" )
@@ -2445,6 +2490,7 @@ def AddFaceToFile( locn_data, face_data, file_eid, model_id, settings ):
# can only be 1 match with the * being a UUID # can only be 1 match with the * being a UUID
fname=glob.glob( f'{path}0_{t.name}_*' )[0] fname=glob.glob( f'{path}0_{t.name}_*' )[0]
new_fname=f'{path}{face.id}_{t.name}' new_fname=f'{path}{face.id}_{t.name}'
# SAFE: as SafePaths(mpath) combined with data I control in this func
os.replace( fname, new_fname ) os.replace( fname, new_fname )
except Exception as ex: except Exception as ex:
print( f"ERROR: AddFaceToFile-face connects to 'disconnected-no-match' metadata, but fixing the filesystem metadata failed: {ex}" ) print( f"ERROR: AddFaceToFile-face connects to 'disconnected-no-match' metadata, but fixing the filesystem metadata failed: {ex}" )

8
settings.py
View File

@@ -185,8 +185,8 @@ def SettingsMPath():
if not settings or settings.metadata_path == "": if not settings or settings.metadata_path == "":
print ("WARNING: no Settings for metadata path") print ("WARNING: no Settings for metadata path")
return return
p=settings.metadata_path if settings.metadata_path[0] == '/':
if p[0] == '/': path=settings.metadata_path
return p
else: else:
return settings.base_path+p path=settings.base_path+settings.metadata_path
return path