Skip to content

Commit 83193b5

Browse files
committed
fix harmful shell injections from graphml
1 parent 58fc130 commit 83193b5

1 file changed

Lines changed: 16 additions & 22 deletions

File tree

mkconcore.py

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -73,19 +73,18 @@
7373
import numpy as np
7474
import shlex # Added for POSIX shell escaping
7575

76-
# --- SECURITY FIX: Input Validation Helper ---
76+
# input validation helper
7777
def safe_name(value, context):
7878
"""
7979
Validates that the input string does not contain characters dangerous
8080
for filesystem paths or shell command injection.
8181
"""
8282
if not value:
8383
raise ValueError(f"{context} cannot be empty")
84-
# Blocks path traversal (/, \\) and shell metacharacters (*, ?, <, >, |, ;, &, $, `)
84+
#blocks path traversal (/, \\) and shell metacharacters (*, ?, <, >, |, ;, &, $, `)
8585
if re.search(r'[\\/:*?"<>|;&`$]', value):
8686
raise ValueError(f"Unsafe {context}: '{value}' contains illegal characters.")
8787
return value
88-
# ---------------------------------------------
8988

9089
MKCONCORE_VER = "22-09-18"
9190

@@ -217,15 +216,13 @@ def safe_name(value, context):
217216
node_label = prefixedgenode + node_label_tag.text
218217
node_label = re.sub(r'(\s+|\n)', ' ', node_label)
219218

220-
# --- SECURITY FIX: Validate Node Labels ---
221-
# Check for malicious characters in container name and source file
219+
#Validate node labels
222220
if ':' in node_label:
223221
container_part, source_part = node_label.split(':', 1)
224222
safe_name(container_part, f"Node container name '{container_part}'")
225223
safe_name(source_part, f"Node source file '{source_part}'")
226224
else:
227225
safe_name(node_label, f"Node label '{node_label}'")
228-
# ------------------------------------------
229226

230227
nodes_dict[node['id']] = node_label
231228
node_id_to_label_map[node['id']] = node_label.split(':')[0]
@@ -243,10 +240,8 @@ def safe_name(value, context):
243240
# Filter out ZMQ edges from the file-based edge dictionary by checking the raw label
244241
if not edge_label_regex.match(raw_label):
245242

246-
# --- SECURITY FIX: Validate Edge Labels ---
247-
# These labels become directory/volume names. Strict validation required.
243+
#Validate edge labels
248244
safe_name(edge_label, f"Edge label '{edge_label}'")
249-
# ------------------------------------------
250245

251246
if edge_label not in edges_dict:
252247
edges_dict[edge_label] = [nodes_dict[edge['source']], []]
@@ -693,7 +688,7 @@ def safe_name(value, context):
693688
for node in nodes_dict:
694689
containername,sourcecode = nodes_dict[node].split(':')
695690
if len(sourcecode)!=0:
696-
safe_container = shlex.quote(containername) # Added safety
691+
safe_container = shlex.quote(containername)
697692
if sourcecode.find(".")==-1:
698693
logging.debug(f"Generating Docker run command: {DOCKEREXE} run --name={containername+volswr[i]+volsro[i]} {DOCKEREPO}/docker- {sourcecode}")
699694
# Use safe_container
@@ -713,7 +708,7 @@ def safe_name(value, context):
713708
if len(sourcecode)!=0:
714709
#dockername,langext = sourcecode.split(".")
715710
dockername = sourcecode.split(".")[0] # 3/28/21
716-
safe_container = shlex.quote(containername) # Added safety
711+
safe_container = shlex.quote(containername)
717712
fstop.write(DOCKEREXE+' stop '+safe_container+"\n")
718713
fstop.write(DOCKEREXE+' rm '+safe_container+"\n")
719714
i=i+1
@@ -746,8 +741,8 @@ def safe_name(value, context):
746741
writeedges = volswr[i]
747742
while writeedges.find(":") != -1:
748743
fmaxtime.write(' -v ')
749-
fmaxtime.write(writeedges.split(":")[0].split("-v ")[1].strip()+":/") # Added strip()
750-
fmaxtime.write(writeedges.split(":")[0].split("-v ")[1].strip()) # Added strip()
744+
fmaxtime.write(writeedges.split(":")[0].split("-v ")[1].strip()+":/")
745+
fmaxtime.write(writeedges.split(":")[0].split("-v ")[1].strip())
751746
writeedges = writeedges[writeedges.find(":")+1:]
752747
i=i+1
753748
fmaxtime.write(' docker-concore >/dev/null &\n')
@@ -761,7 +756,7 @@ def safe_name(value, context):
761756
writeedges = volswr[i]
762757
while writeedges.find(":") != -1:
763758
fmaxtime.write('sudo docker cp concore.maxtime concore:/')
764-
fmaxtime.write(writeedges.split(":")[0].split("-v ")[1].strip()+"/concore.maxtime\n") # Added strip()
759+
fmaxtime.write(writeedges.split(":")[0].split("-v ")[1].strip()+"/concore.maxtime\n")
765760
writeedges = writeedges[writeedges.find(":")+1:]
766761
i=i+1
767762
fmaxtime.write('sudo docker stop concore \n')
@@ -785,8 +780,8 @@ def safe_name(value, context):
785780
writeedges = volswr[i]
786781
while writeedges.find(":") != -1:
787782
fparams.write(' -v ')
788-
fparams.write(writeedges.split(":")[0].split("-v ")[1].strip()+":/") # Added strip()
789-
fparams.write(writeedges.split(":")[0].split("-v ")[1].strip()) # Added strip()
783+
fparams.write(writeedges.split(":")[0].split("-v ")[1].strip()+":/")
784+
fparams.write(writeedges.split(":")[0].split("-v ")[1].strip())
790785
writeedges = writeedges[writeedges.find(":")+1:]
791786
i=i+1
792787
fparams.write(' docker-concore >/dev/null &\n')
@@ -800,7 +795,7 @@ def safe_name(value, context):
800795
writeedges = volswr[i]
801796
while writeedges.find(":") != -1:
802797
fparams.write('sudo docker cp concore.params concore:/')
803-
fparams.write(writeedges.split(":")[0].split("-v ")[1].strip()+"/concore.params\n") # Added strip()
798+
fparams.write(writeedges.split(":")[0].split("-v ")[1].strip()+"/concore.params\n")
804799
writeedges = writeedges[writeedges.find(":")+1:]
805800
i=i+1
806801
fparams.write('sudo docker stop concore \n')
@@ -823,8 +818,8 @@ def safe_name(value, context):
823818
writeedges = volswr[i]
824819
while writeedges.find(":") != -1:
825820
funlock.write(' -v ')
826-
funlock.write(writeedges.split(":")[0].split("-v ")[1].strip()+":/") # Added strip()
827-
funlock.write(writeedges.split(":")[0].split("-v ")[1].strip()) # Added strip()
821+
funlock.write(writeedges.split(":")[0].split("-v ")[1].strip()+":/")
822+
funlock.write(writeedges.split(":")[0].split("-v ")[1].strip())
828823
writeedges = writeedges[writeedges.find(":")+1:]
829824
i=i+1
830825
funlock.write(' docker-concore >/dev/null &\n')
@@ -838,7 +833,7 @@ def safe_name(value, context):
838833
writeedges = volswr[i]
839834
while writeedges.find(":") != -1:
840835
funlock.write('sudo docker cp ~/concore.apikey concore:/')
841-
funlock.write(writeedges.split(":")[0].split("-v ")[1].strip()+"/concore.apikey\n") # Added strip()
836+
funlock.write(writeedges.split(":")[0].split("-v ")[1].strip()+"/concore.apikey\n")
842837
writeedges = writeedges[writeedges.find(":")+1:]
843838
i=i+1
844839
funlock.write('sudo docker stop concore \n')
@@ -1173,5 +1168,4 @@ def safe_name(value, context):
11731168
os.chmod(outdir+"/clear",stat.S_IRWXU)
11741169
os.chmod(outdir+"/maxtime",stat.S_IRWXU)
11751170
os.chmod(outdir+"/params",stat.S_IRWXU)
1176-
os.chmod(outdir+"/unlock",stat.S_IRWXU)
1177-
1171+
os.chmod(outdir+"/unlock",stat.S_IRWXU)

0 commit comments

Comments
 (0)