add: добавляет гипнофлеш драфт#8567
Conversation
|
классная аватарка |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new hypnotic flash device. A security review identified a critical Cross-Site Scripting (XSS) vulnerability in the flash_carbon proc, where object names are interpolated into HTML-formatted chat messages without proper sanitization, potentially allowing attackers to execute arbitrary JavaScript. The code review also noted a potential null object dereference at code/game/objects/items/devices/flash.dm:341 and suggested improvements for localization and consistent return values within the flash_carbon procedure.
| if(user) | ||
| add_attack_logs(user, flashed, "[targeted? "hypno-flashed(targeted)" : "hypno-flashed(AOE)"] [extra_log]", src) | ||
| else //caused by emp/remote signal | ||
| add_attack_logs("was [targeted? "hypno-flashed(targeted)" : "hypno-flashed(AOE)"] [extra_log]", ATTACK_CHAIN_PROCEED) |
There was a problem hiding this comment.
🔴 Критическая ошибка: отсутствует проверка на null
В процедуре flash_carbon отсутствует проверка на null для переменной flashed перед её использованием. Если в процедуру будет передано null, это приведёт к ошибке выполнения (runtime error).
Согласно стайлгайду (правило 97), необходимо всегда проверять на null объекты, особенно полученные в качестве аргументов.
Рекомендую добавить проверку в самом начале процедуры.
if(!flashed)
return
if(user)
add_attack_logs(user, flashed, "[targeted? "hypno-flashed(targeted)" : "hypno-flashed(AOE)"] [extra_log]", src)
else //caused by emp/remote signal
add_attack_logs("was [targeted? "hypno-flashed(targeted)" : "hypno-flashed(AOE)"] [extra_log]", ATTACK_CHAIN_PROCEED)
References
- Требовать проверку на
null(if(!variable)) перед использованием объекта, особенно послеlocate(),find_in_view()и т.п. (link)
| to_chat(flashed, span_notice("[src] emits a soothing light...")) | ||
|
|
||
| if(!flashed.flash_eyes(1, override_blindness_check = targeted, affect_silicon = TRUE)) | ||
| if(targeted) | ||
| if(user) | ||
| user.visible_message(span_warning("[user] fails to blind [flashed] with the flash!"), span_warning("You fail to hypno-flash [flashed]!")) | ||
| else | ||
| to_chat(flashed, span_danger("[src] fails to blind you!")) | ||
| return FALSE | ||
|
|
||
| if(!targeted) | ||
| to_chat(flashed, span_notice("Such a pretty light...")) | ||
| flashed.AdjustConfused(confusion_duration, confusion_duration * 2) | ||
| flashed.AdjustDizzy(8 SECONDS, 40 SECONDS) | ||
| flashed.AdjustDrowsy(8 SECONDS, 40 SECONDS) | ||
| return TRUE | ||
|
|
||
| if(user) | ||
| user.visible_message(span_danger("[user] blinds [flashed] with the flash!"), span_danger("You hypno-flash [flashed]!")) | ||
| else | ||
| to_chat(flashed, "You're blinded by [src]!") |
There was a problem hiding this comment.
A critical Cross-Site Scripting (XSS) vulnerability exists here. The flash_carbon proc interpolates object names (src, user, flashed) directly into HTML-generating macros (span_notice, span_warning, span_danger) without proper sanitization. This allows attackers to inject malicious HTML or JavaScript by modifying item or mob names, leading to XSS in other players' chat boxes. To mitigate this, use html_encode() for any dynamic content interpolated into HTML strings, for example: to_chat(flashed, span_notice("[html_encode(src.name)] emits a soothing light...")). Additionally, this block has localization issues, as messages for players should be in Russian and use localization functions (rules 103, 115). The flash_carbon procedure also has inconsistent return values, as it does not return a value in some branches while others return TRUE or FALSE.
References
- Messages for players should be in Russian and use localization functions (rules 103, 115).
|
|
||
|
|
||
| /obj/item/flash/hypnotic | ||
| desc = "A modified flash device, programmed to emit a sequence of subliminal flashes that can send a vulnerable target into a hypnotic trance." |
There was a problem hiding this comment.
🟡 Рекомендация по локализации
Описание предмета (desc) должно быть на русском языке для корректного отображения в игре. Текущее описание на английском.
Согласно стайлгайду (правило 103), описания для игроков должны быть локализованы.
desc = "Модифицированный флешер, запрограммированный на излучение последовательности подсознательных вспышек, способных ввести уязвимую цель в гипнотический транс."
References
- Имя объекта (
name) у/objне переводится. Для русских названий используется система ru_names. Описания (desc) должны быть на русском языке. (link)
pilygun
added
the
📜 Локализация
|
Описание перевести обязательно, Так же обязательно сделать Ру неймы |
3 participants
Что этот ПР делает
добавляет гипнофлеш
Почему это хорошо для игры
Демонстрация изменений
Демонстрации изменений
Тестирование