Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion MaiChartManager/Controllers/Charts/ChartController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,29 @@ public void EditChartEnable(int id, int level, [FromBody] bool value, string ass
}

[HttpPost]
public void ReplaceChart(int id, int level, IFormFile file, string assetDir)
public void ReplaceChart(int id, int level, IFormFile file, string assetDir,
[FromForm] ImportChartController.ShiftMethod? shift)
Comment on lines +98 to +99

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The assetDir parameter, which is a route parameter and thus user-controlled, is used to construct file paths using Path.Combine (e.g., on line 106) without any validation or sanitization. An attacker can provide a path containing directory traversal sequences (e.g., ..) to escape the intended directory and overwrite arbitrary files on the server where the application has write permissions.

Remediation: Validate the assetDir parameter against an allowlist of expected directory names or sanitize it to remove any path traversal sequences before using it in file system operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clansty 这个用管吗😰 虽然他说有问题的这段并不是我写的,我只是加了个shift参数而已(

{
var music = settings.GetMusic(id, assetDir);
if (music == null || file == null) return;
// TODO 判断是MA2还是maidata.txt,走不同的逻辑
var targetChart = music.Charts[level];
targetChart.Path = $"{id:000000}_0{level}.ma2";
using var stream = System.IO.File.Open(Path.Combine(StaticSettings.StreamingAssets, assetDir, "music", $"music{id:000000}", targetChart.Path), FileMode.Create);
file.CopyTo(stream);
targetChart.Problems.Clear();
stream.Close();

// 检查新谱面ma2的音符数量是否有变化,如果有修正之
string fileContent;
using (var reader = new StreamReader(file.OpenReadStream()))
{
fileContent = reader.ReadToEnd();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The ReplaceChart method at this line reads the entire content of an uploaded file into memory using reader.ReadToEnd() without any size validation. This creates a Denial of Service (DoS) risk, as a large file could lead to an OutOfMemoryException and application crash, especially when passed to ParseTNumAllFromMa2. Additionally, the current implementation has two code quality issues: it attempts to read the file stream multiple times (file.CopyTo(stream) then file.OpenReadStream()), which can cause errors with non-seekable streams, and it includes a redundant stream.Close() within a using statement. It is recommended to implement a maximum file size limit, process files using a stream-based approach, and refactor to read the file content only once for both writing and parsing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个不用太管吧,后端是在用户自己的电脑上运行的,我DoS我自己吗?

}
var newMaxNotes = ImportChartController.ParseTNumAllFromMa2(fileContent);
if (newMaxNotes != 0 && targetChart.MaxNotes != newMaxNotes)
{
targetChart.MaxNotes = newMaxNotes;
}
}
}
12 changes: 10 additions & 2 deletions MaiChartManager/Controllers/Charts/ImportChartController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,18 @@ public record ImportChartMessage(string Message, MessageLevel Level);
public record ImportChartCheckResult(bool Accept, IEnumerable<ImportChartMessage> Errors, float MusicPadding, bool IsDx, string? Title, float first, float bar);

[HttpPost]
public ImportChartCheckResult ImportChartCheck(IFormFile file)
public ImportChartCheckResult ImportChartCheck(IFormFile file, [FromForm] bool isReplacement = false)
{
var errors = new List<ImportChartMessage>();
var fatal = false;

if (isReplacement)
{
// 替换谱面的操作也需要检查的过程,但检查的逻辑和导入谱面时可以说是一模一样的,故直接共用逻辑
// 唯一的区别是给用户一个警告,明确说明直接替换谱面功能的适用范围
errors.Add(new ImportChartMessage(Locale.NotesReplacementWarning, MessageLevel.Warning));
}

try
{
var kvps = new SimaiFile(file.OpenReadStream()).ToKeyValuePairs();
Expand Down Expand Up @@ -559,6 +566,7 @@ public ImportChartResult ImportChart(
music.AddVersionId = addVersionId;
music.GenreId = genreId;
music.Version = version;
music.ShiftMethod = shift.ToString();
float wholebpm;
if (float.TryParse(maiData.GetValueOrDefault("wholebpm"), out wholebpm))
music.Bpm = wholebpm;
Expand All @@ -568,7 +576,7 @@ public ImportChartResult ImportChart(
}


private static int ParseTNumAllFromMa2(string ma2Content)
public static int ParseTNumAllFromMa2(string ma2Content)
{
var lines = ma2Content.Split('\n');
// 从后往前读取,因为 T_NUM_ALL 在文件最后
Expand Down
4 changes: 4 additions & 0 deletions MaiChartManager/Front/src/client/apiGen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ export interface MusicXmlWithABJacket {
subLockType?: number;
disable?: boolean;
longMusic?: boolean;
shiftMethod?: string | null;
charts?: Chart[] | null;
assetBundleJacket?: string | null;
pseudoAssetBundleJacket?: string | null;
Expand Down Expand Up @@ -965,6 +966,7 @@ export class Api<SecurityDataType extends unknown> extends HttpClient<SecurityDa
data: {
/** @format binary */
file?: File;
shift?: ShiftMethod;
},
params: RequestParams = {},
) =>
Expand Down Expand Up @@ -1236,6 +1238,8 @@ export class Api<SecurityDataType extends unknown> extends HttpClient<SecurityDa
data: {
/** @format binary */
file?: File;
/** @default false */
isReplacement?: boolean;
},
params: RequestParams = {},
) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,68 @@
import { t } from '@/locales';
import { globalCapture, selectedADir, selectedLevel, selectedMusic, selectMusicId, updateMusicList } from '@/store/refs';
import { NButton, NFlex, NModal, useMessage } from 'naive-ui';
import { defineComponent, PropType, ref, computed, watch, shallowRef } from 'vue';
import { NButton, NFlex, NModal, useDialog, useMessage } from 'naive-ui';
import { defineComponent, ref, shallowRef } from 'vue';
import JacketBox from '../JacketBox';
import { DIFFICULTY, LEVEL_COLOR } from '@/consts';
import api from '@/client/api';
import CheckingModal from "@/components/ImportCreateChartButton/ImportChartButton/CheckingModal";

export const replaceChartFileHandle = shallowRef<FileSystemFileHandle | null>(null);
export let prepareReplaceChart = async (fileHandle?: FileSystemFileHandle) => {
}

export default defineComponent({
// props: {
// },
setup(props, { emit }) {
const message = useMessage();
const dialog = useDialog();

const replaceChart = async () => {
if (!replaceChartFileHandle.value) return;
const checking = ref(false);
const ma2Handle = shallowRef<FileSystemFileHandle | null>(null);

prepareReplaceChart = async (fileHandle?: FileSystemFileHandle) => {
if (!fileHandle) {
[fileHandle] = await window.showOpenFilePicker({
id: 'chart',
startIn: 'downloads',
types: [
{
description: t('music.edit.supportedFileTypes'),
accept: {
"application/x-supported": [".ma2", ".txt"], // 没办法限定只匹配maidata.txt,就只好先把一切txt都作为匹配
},
},
],
});
}
if (!fileHandle) return; // 用户未选择文件

const name = fileHandle.name;
// 对maidata.txt和ma2分类讨论,前者执行ImportCheck
if (name == "maidata.txt") {
try {
checking.value = true;
const file = await fileHandle.getFile();
const checkRet = (await api.ImportChartCheck({file, isReplacement: true})).data;
if (!checking.value) return; // 说明检查期间用户点击了关闭按钮、取消了操作。则不再执行后续流程。
// TODO 显示导入界面(类似ErrorDisplayIdInput)、完成导入流程
console.log(checkRet)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

这里有一个 console.log 语句,可能是用于调试的。在代码最终合并之前,建议移除这些调试相关的日志输出,以保持生产环境代码的整洁。

dialog.error({title: "NotImplemented"})
} finally {
checking.value = false;
}
} else if (name.endsWith(".ma2")) {
ma2Handle.value = fileHandle
} else {
dialog.error({title: t('error.unsupportedFileType'), content: t('music.edit.notValidChartFile')})
}
}

const replaceMa2 = async () => {
if (!ma2Handle.value) return;
try {
const file = await replaceChartFileHandle.value.getFile();
replaceChartFileHandle.value = null;
const file = await ma2Handle.value.getFile();
ma2Handle.value = null;
await api.ReplaceChart(selectMusicId.value, selectedLevel.value, selectedADir.value, { file });
message.success(t('music.edit.replaceChartSuccess'));
await updateMusicList();
Expand All @@ -28,34 +72,37 @@ export default defineComponent({
}
}

return () => <NModal
preset="card"
class="w-[min(90vw,50em)]"
title={t('music.edit.replaceChart')}
show={replaceChartFileHandle.value !== null}
onUpdateShow={() => replaceChartFileHandle.value = null}
>{{
default: () => <div class="flex flex-col gap-2">
{t('music.edit.replaceChartConfirm', { level: DIFFICULTY[selectedLevel.value!] })}
<div class="text-4.5 text-center">{replaceChartFileHandle.value?.name}</div>
<div class="text-6 text-center">↓</div>
<div class="flex justify-center gap-2">
<JacketBox info={selectedMusic.value!} class="h-8em w-8em" upload={false} />
<div class="flex flex-col gap-1 max-w-24em justify-end">
<div class="text-3.5 op-70">#{selectMusicId.value}</div>
<div class="text-2xl overflow-hidden text-ellipsis whitespace-nowrap">{selectedMusic.value!.name}</div>
<div class="flex">
<div class="c-white rounded-full px-2" style={{ backgroundColor: LEVEL_COLOR[selectedLevel.value!] }}>
{selectedMusic.value!.charts![selectedLevel.value!]?.level}.{selectedMusic.value!.charts![selectedLevel.value!]?.levelDecimal}
return () => <div>
<NModal
preset="card"
class="w-[min(90vw,50em)]"
title={t('music.edit.replaceChart')}
show={ma2Handle.value !== null}
onUpdateShow={() => ma2Handle.value = null}
>{{
default: () => <div class="flex flex-col gap-2">
{t('music.edit.replaceChartConfirm', { level: DIFFICULTY[selectedLevel.value!] })}
<div class="text-4.5 text-center">{ma2Handle.value?.name}</div>
<div class="text-6 text-center">↓</div>
<div class="flex justify-center gap-2">
<JacketBox info={selectedMusic.value!} class="h-8em w-8em" upload={false} />
<div class="flex flex-col gap-1 max-w-24em justify-end">
<div class="text-3.5 op-70">#{selectMusicId.value}</div>
<div class="text-2xl overflow-hidden text-ellipsis whitespace-nowrap">{selectedMusic.value!.name}</div>
<div class="flex">
<div class="c-white rounded-full px-2" style={{ backgroundColor: LEVEL_COLOR[selectedLevel.value!] }}>
{selectedMusic.value!.charts![selectedLevel.value!]?.level}.{selectedMusic.value!.charts![selectedLevel.value!]?.levelDecimal}
</div>
</div>
</div>
</div>
</div>
</div>,
footer: () => <NFlex justify="end">
<NButton onClick={() => replaceChartFileHandle.value = null}>{t('common.cancel')}</NButton>
<NButton onClick={replaceChart} type="primary">{t('common.confirm')}</NButton>
</NFlex>
}}</NModal>;
</div>,
footer: () => <NFlex justify="end">
<NButton onClick={() => ma2Handle.value = null}>{t('common.cancel')}</NButton>
<NButton onClick={replaceMa2} type="primary">{t('common.confirm')}</NButton>
</NFlex>
}}</NModal>
<CheckingModal title={t('chart.import.checkingTitle')} show={checking.value} closeModal={()=>checking.value=false} />
</div>;
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { uploadFlow as uploadFlowMovie } from '@/components/MusicEdit/SetMovieBu
import { uploadFlow as uploadFlowAcbAwb } from '@/components/MusicEdit/AcbAwb';
import { selectedADir, selectedMusic } from '@/store/refs';
import { upload as uploadJacket } from '@/components/JacketBox';
import ReplaceChartModal, { replaceChartFileHandle } from './ReplaceChartModal';
import ReplaceChartModal, { prepareReplaceChart } from './ReplaceChartModal';
import AquaMaiManualInstaller, { setManualInstallAquaMai } from './AquaMaiManualInstaller';

export const mainDivRef = shallowRef<HTMLDivElement>();
Expand Down Expand Up @@ -48,8 +48,8 @@ export default defineComponent({
else if (file.kind === 'file' && (firstType.startsWith('image/') || file.name.endsWith('.jpeg') || file.name.endsWith('.jpg') || file.name.endsWith('.png'))) {
uploadJacket(file);
}
else if (file.kind === 'file' && file.name.endsWith('.ma2')) {
replaceChartFileHandle.value = file;
else if (file.kind === 'file' && (file.name.endsWith('.ma2') || file.name == "maidata.txt")) {
prepareReplaceChart(file);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { computed, defineComponent, PropType, watch } from "vue";
import { Chart } from "@/client/apiGen";
import { NFlex, NForm, NFormItem, NInput, NInputNumber, NSelect, NSwitch } from "naive-ui";
import { NButton, NFlex, NForm, NFormItem, NInput, NInputNumber, NSelect, NSwitch } from "naive-ui";
import api from "@/client/api";
import { selectedADir, selectedMusic } from "@/store/refs";
import { LEVELS } from "@/consts";
import ProblemsDisplay from "@/components/ProblemsDisplay";
import PreviewChartButton from "@/components/MusicEdit/PreviewChartButton";
import { useI18n } from 'vue-i18n';
import { prepareReplaceChart } from "@/components/DragDropDispatcher/ReplaceChartModal";

const LEVELS_OPTIONS = LEVELS.map((level, index) => ({label: level, value: index}));

Expand Down Expand Up @@ -43,6 +44,9 @@ export default defineComponent({
<NFlex vertical>
<NFlex align="center" class="absolute right-0 top-0 m-xy mt-2 z-2">
<PreviewChartButton songId={props.songId} level={props.chartIndex}/>
<NButton secondary onClick={() => prepareReplaceChart()}>
{t('music.edit.replaceChart')}
</NButton>
</NFlex>
<NFormItem label={t('music.edit.chartEnable')} labelPlacement="left" class="ml-2px">
<NFlex align="center">
Expand Down
2 changes: 2 additions & 0 deletions MaiChartManager/Front/src/locales/en.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ music:
replaceChartConfirm: Confirm to replace {level}?
replaceChartFailed: Failed to replace chart
replaceChartSuccess: Chart replaced successfully
notValidChartFile: Chart file must be .ma2 or maidata.txt.
batch:
title: Batch Actions
batchAndSearch: Batch Actions & Search
Expand Down Expand Up @@ -456,6 +457,7 @@ error:
confirm: Got it
file:
notSelected: No file selected
unsupportedFileType: 'Unsupported file type'
message:
notice: Notice
saveSuccess: Saved successfully
Expand Down
2 changes: 2 additions & 0 deletions MaiChartManager/Front/src/locales/zh-TW.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ music:
replaceChartConfirm: 確認要替換 {level} 譜面嗎?
replaceChartFailed: 替換譜面失敗
replaceChartSuccess: 替換譜面成功
notValidChartFile: 譜面檔案必須是 .ma2 或 maidata.txt。
batch:
title: 批次操作
batchAndSearch: 批次操作與搜尋
Expand Down Expand Up @@ -417,6 +418,7 @@ error:
feedbackError: 回饋錯誤
file:
notSelected: 未選擇檔案
unsupportedFileType: '不支援的檔案類型'
message:
notice: 提示
saveSuccess: 儲存成功
Expand Down
2 changes: 2 additions & 0 deletions MaiChartManager/Front/src/locales/zh.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ music:
replaceChartConfirm: 确认要替换 {level} 谱面吗?
replaceChartFailed: 替换谱面失败
replaceChartSuccess: 替换谱面成功
notValidChartFile: 谱面文件必须是.ma2或maidata.txt
batch:
title: 批量操作
batchAndSearch: 批量操作与搜索
Expand Down Expand Up @@ -418,6 +419,7 @@ error:
feedbackError: 反馈错误
file:
notSelected: 未选择文件
unsupportedFileType: 不支持的文件类型
message:
notice: 提示
saveSuccess: 保存成功
Expand Down
9 changes: 9 additions & 0 deletions MaiChartManager/Locale.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions MaiChartManager/Locale.resx
Original file line number Diff line number Diff line change
Expand Up @@ -293,4 +293,7 @@ If you notice any issues with the conversion result, you can try testing it in A
<data name="LauncherOpenToLanDesc" xml:space="preserve">
<value>Automatically run in server mode and minimize to the system tray upon startup.</value>
</data>
<data name="NotesReplacementWarning" xml:space="preserve">
<value>Caution! This "Replace Chart" function should only be used when modifying the note content of the existing chart, while the audio remains unchanged, the &amp;first offset remains unchanged, and the timing of the first note remains unchanged. Otherwise, you will need to delete the entire chart and re-import it.</value>
</data>
</root>
3 changes: 3 additions & 0 deletions MaiChartManager/Locale.zh-hans.resx
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,7 @@
<data name="LauncherOpenToLanDesc" xml:space="preserve">
<value>开机自动以服务器模式运行并最小化到托盘</value>
</data>
<data name="NotesReplacementWarning" xml:space="preserve">
<value>注意!本“替换谱面”功能仅限用于:谱面音符内容在原来的基础上发生修改,且音频内容未变、&amp;first偏移量未变、谱面中第一个音符的时刻未变的情况。否则,您需要删除整个谱面后重新导入。</value>
</data>
</root>
3 changes: 3 additions & 0 deletions MaiChartManager/Locale.zh-hant.resx
Original file line number Diff line number Diff line change
Expand Up @@ -285,4 +285,7 @@
<data name="LauncherOpenToLanDesc" xml:space="preserve">
<value>開機自動以伺服器模式運作並最小化到托盤</value>
</data>
<data name="NotesReplacementWarning" xml:space="preserve">
<value>>請注意!本「替換譜面」功能僅限用於:譜面音符內容在原來的基礎上發生修改,且音訊內容未變、&amp;first偏移量未變、譜面中第一個音符的時刻未變的情況。否則,您需要刪除整個譜面後重新匯入。</value>
</data>
</root>
Loading