Skip to content

Commit e7c7562

Browse files
committed
fixing a bug spotted in codeql
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
1 parent 0e18848 commit e7c7562

3 files changed

Lines changed: 55 additions & 2 deletions

File tree

CHANGELOG.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ Native multi agents support:
102102

103103
[1.12.5] - 2026-05-xx
104104
-------------------------
105+
- [FIXED] some security quality issue spotted by codeQL (I/O function call with non totally checked user input)
105106
- [ADDED] automatic release on pypi
106107
- [ADDED] a file
107108
- [IMPROVED] clarifying the CONTRIBUTING.md document

grid2op/tests/test_utils.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import copy
1010
import json
1111
import os
12+
import re
1213
from hashlib import blake2b
1314
from unittest.mock import patch
1415
import numpy as np
@@ -614,6 +615,42 @@ def test_regex_rejects_path_traversal(self):
614615

615616
scores.clear_all()
616617

618+
def test_dotdot_passes_regex_but_containment_check_blocks_it(self):
619+
"""Layer 3 (realpath containment check, fix for S2083).
620+
621+
A bare '..' has no '/' so it passes REGEX_SPLIT, which only blocks
622+
multi-component paths like '../../etc/passwd'. Before the fix,
623+
os.path.join(path_save, '..', META_FILE) would silently read a file
624+
one directory above path_save. The realpath containment check now
625+
catches this case.
626+
"""
627+
# First, confirm that '..' passes the regex — documenting WHY the regex
628+
# alone is not sufficient.
629+
self.assertIsNotNone(
630+
re.match(EpisodeStatistics.REGEX_SPLIT_COMPILED, ".."),
631+
"'..' contains only dots which are in the allowed charset — "
632+
"regex alone cannot block single-component traversal",
633+
)
634+
635+
with warnings.catch_warnings():
636+
warnings.filterwarnings("ignore")
637+
with grid2op.make(
638+
"rte_case5_example", test=True, _add_to_name=type(self).__name__
639+
) as env:
640+
scores = ScoreL2RPN2020(env, nb_scenario=2, verbose=0, max_step=10)
641+
my_agent = DoNothingAgent(env.action_space)
642+
643+
malicious_meta = copy.deepcopy(scores.stat_dn.get_metadata())
644+
malicious_meta["0"]["scenario_name"] = ".."
645+
646+
with patch.object(
647+
scores.stat_dn, "get_metadata", return_value=malicious_meta
648+
):
649+
with self.assertRaises(RuntimeError):
650+
scores.get(my_agent)
651+
652+
scores.clear_all()
653+
617654

618655
if __name__ == "__main__":
619656
unittest.main()

grid2op/utils/l2rpn_2020_scores.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,20 +383,35 @@ def get(self, agent, path_save=None, nb_process=1):
383383
all_scores = []
384384
ts_survived = []
385385
total_ts = []
386+
path_save_real = os.path.realpath(path_save)
387+
# Build a map from name → absolute path using the real filesystem so
388+
# that the tainted scenario_name from the metadata JSON is never used
389+
# directly as a path component (satisfies CodeQL py/path-injection).
390+
available_ep_dirs = {
391+
entry: os.path.join(path_save_real, entry)
392+
for entry in os.listdir(path_save_real)
393+
if os.path.isdir(os.path.join(path_save_real, entry))
394+
}
386395
for ep_id in range(self.nb_scenario):
387396
this_ep_nm = meta_data_dn[f"{ep_id}"]["scenario_name"]
388397
if re.match(EpisodeStatistics.REGEX_SPLIT_COMPILED, this_ep_nm) is None:
389398
raise RuntimeError(
390399
f'The grid2op statistics name should match the regex "{EpisodeStatistics.REGEX_SPLIT}", it is currently {this_ep_nm}.'
391400
)
401+
if this_ep_nm not in available_ep_dirs:
402+
raise RuntimeError(
403+
f"Episode directory for scenario '{this_ep_nm}' was not found under {path_save}."
404+
)
405+
# ep_dir comes from os.listdir (filesystem), not from the tainted metadata value.
406+
ep_dir = available_ep_dirs[this_ep_nm]
392407
with open(
393-
os.path.join(path_save, this_ep_nm, EpisodeData.META_FILE),
408+
os.path.join(ep_dir, EpisodeData.META_FILE),
394409
"r",
395410
encoding="utf-8",
396411
) as f:
397412
this_epi_meta = json.load(f)
398413
with open(
399-
os.path.join(path_save, this_ep_nm, EpisodeData.OTHER_REWARDS_FILE),
414+
os.path.join(ep_dir, EpisodeData.OTHER_REWARDS_FILE),
400415
"r",
401416
encoding="utf-8",
402417
) as f:

0 commit comments

Comments
 (0)