Potential integer overflow in NLog.Targets.FileTarget.WriteToMemoryStream while calculating BufferSize * 100
维护者通常 1 天内回复
已经有一个关联 PR 被合并了。
- #6325 来自 @snakefoot —— 已合并
评估
调研方向
溢出位于 FileTarget 源码 (NLog/Targets/FileTarget.cs) 中的 WriteToMemoryStream,在计算 maxBufferSize(由 BufferSize * 100 得出)的那一行。阅读该方法以了解 maxBufferSize 如何使用,然后确认乘法在扩展到 long 之前是以 32 位执行的。当乘法以 64 位算术执行且现有 FileTarget 测试仍然通过时完成。
由索引模型根据 Issue 内容生成。
描述
NLog version: 6.1.3 (and it is also present in the current dev branch)
Platform: All supported platforms
Current NLog config (xml or C#, if relevant)
Not relevant. The possible issue has been detected during analysis of the code.
- What is the current result?
Dear NLog maintainers!
It seems that there might be a potential integer overflow issue in the NLog.Targets.FileTarget.WriteToMemoryStream method. Please have a look at the following lines of code:
private int WriteToMemoryStream(IList<AsyncLogEventInfo> logEvents, int startIndex, MemoryStream ms)
{
long maxBufferSize = BufferSize * 100; // Max Buffer Default = 30 KiloByte * 100 = 3 MegaByte
//...
}
The current implementation performs a 32-bit integer multiplication (BufferSize * 100) due to the fact that BufferSize and 100 are both integer. Only after that the statement casts the result into a wider 64-bit long type. Unfortunately, if the BufferSize holds a large enough value, an overflow will occur before having the result implicitly casted to long and assigned to maxBufferSize.
-
What is the expected result?
It looks like we should compute the mentioned multiplication using a 64 instead of a 32-bit arithmetic in order to avoid the risk of 32-bit integer overflow. -
Possible fixes:
Could we consider the following possible changes or introduce another one to solve that issue?
Option1 (using L suffix):
long maxBufferSize = BufferSize * 100L; // Max Buffer Default = 30 KiloByte * 100 = 3 MegaByte
Option2 (state the intention explicitly):
long maxBufferSize = (long)BufferSize * 100;
-
Did you checked the Internal log?
Not relevant. -
Please post full exception details
There is no exceptions under normal conditions. Analysis of the implementation shows that the code of the whole method is stable enough even if we get the issue to fire. So, we could consider that as a minor but confirmed bug that leads to the behaviour that is not expected. -
Are there any workarounds?
Yes, keepingBufferSizewithin standard limits prevents actual application crashes, but the code quality issue remains. -
Is there a version in which it did work?
It looks like this arithmetic expression has been here for a while. -
Can you help us by writing an unit test?
A unit test is likely not needed as this is a preventive fix.
Note:
I would highly appreciate if this minor fix could be included or backported to the 6.1.3+ branch.
- 主要语言
- C#
- 星标
- 6.5k
- 派生
- 1.4k
- 平均合并
- 15 小时 27 分钟
- 30 天内合并 PR
- 23
环境准备
- 没有 Dockerfile 或 Docker Compose 文件
- 没有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
NLog/NLog 的其他 Issue
-
.NET Matrix未关闭
难度 2/5 1-3 小时 新手友好度 68/100
维护者通常 1 天内回复
-
DateLayoutRenderer default Format should be culture invariant可能已有人在做 @HarnageaGabriel 于 71 天前认领。 未关闭breaking behavior change breaking change
难度 2/5 1-3 小时 新手友好度 65/100
维护者通常 1 天内回复
-
Move away from AppVeyor and Azure Pipelines (SonarQube analysis)可能已有人在做 @304NotModified 于 1 天前认领。 未关闭
难度 4/5 3-5 天 新手友好度 45/100
维护者通常 1 天内回复
-
Switch to trusted publishing (and GitHub actions)可能已有人在做 @304NotModified 于 11 天前认领。 未关闭build-config
NLog/NLog#6306 · 14 条评论 · 已指派 1 人 ·
维护者通常 1 天内回复
-
platform support unit-tests
难度 4/5 3-5 天 新手友好度 52/100
维护者通常 1 天内回复
相似的 Issue
-
area-ai bug untriaged
难度 2/5 1-3 小时 新手友好度 67/100
dotnet/extensions#7893 ·
维护者通常 1 天内回复
-
devtools documentation
难度 2/5 1-3 小时 新手友好度 76/100
维护者通常 1 天内回复
-
area-System.ComponentModel untriaged
难度 2/5 1-3 小时 新手友好度 65/100
dotnet/runtime#136608 · 2 条评论 ·
维护者通常 1 天内回复
-
fix: Cannot reopen dialog after closing with escape key in FluentDialog可能已有人在做 @bryan-ramirez 今天认领。 未关闭
难度 2/5 1-3 小时 新手友好度 72/100
microsoft/fluentui-blazor#5572 · 1 条评论 ·
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 78/100
sillsdev/languageforge-lexbox#2846 ·
维护者通常 1 天内回复