Repository navigation
#52 Add asset store and main site templates (WIP) - #111
Conversation
#40 Fix UI renderer
A lot of stuff is unfinished. Context menu locations, launcher styling, xml documentation missing etc. Functionality should be working tho
Textures still missing and the framerate is very bad
#2 Load models from obj files
|
| public ActionResult<IEnumerable<Asset>> GetAllAssetsByKeyWord([FromBody] IReadOnlyList<string> keywords) | ||
| { | ||
| var joinedKeyWords = string.Join(", ", keywords); | ||
| _logger.LogDebug("Retrieving assets for the given keywords: {KeyWords}.", joinedKeyWords); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix this problem, we need to sanitize keywords before logging. Since the guidance indicates that for plain text logs, newlines and potentially problematic characters should be removed from user input, we should process each keyword to remove (or replace) carriage returns and newlines (\r, \n). This should be done when constructing joinedKeyWords so that the logged string cannot be used to inject artificial log entries or confuse log viewers.
Specifically:
- In the method
GetAllAssetsByKeyWord, when buildingjoinedKeyWords, process each string inkeywordsusing.Replace("\r", "").Replace("\n", "")(or use a helper method/extension if you prefer). - Do not otherwise change the logging or functionality.
- Change only lines involved in the log entry construction within this controller file.
No additional imports or dependencies are needed.
| @@ -44,7 +44,8 @@ | ||
| [HttpGet("/keyword")] | ||
| public ActionResult<IEnumerable<Asset>> GetAllAssetsByKeyWord([FromBody] IReadOnlyList<string> keywords) | ||
| { | ||
| var joinedKeyWords = string.Join(", ", keywords); | ||
| var sanitizedKeywords = keywords.Select(k => k.Replace("\r", "").Replace("\n", "")); | ||
| var joinedKeyWords = string.Join(", ", sanitizedKeywords); | ||
| _logger.LogDebug("Retrieving assets for the given keywords: {KeyWords}.", joinedKeyWords); | ||
|
|
||
| var assets = _service.GetAssets(keywords); |
| _logger.LogDebug("Retrieving assets for the given keywords: {KeyWords}.", joinedKeyWords); | ||
|
|
||
| var assets = _service.GetAssets(keywords); | ||
| _logger.LogDebug("Found {Count} assets for the given keywords: {KeyWords}.", assets.Count(), joinedKeyWords); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the issue, user-provided values used in log entries should be sanitized to prevent log forging. Since the log output is very likely plain text (as is typical for .NET logging), all line breaks (\n, \r), and possibly other control characters, should be removed from the keywords before they are joined and logged. This involves replacing or removing any line break characters in each keyword before joining them into the joinedKeyWords string. The optimal fix is to sanitize each element of the keywords list by replacing/removing line breaks before joining them. That way, even if a malicious user tries to inject line breaks in any keyword, it will not affect the log integrity.
The changes should be localized to the method GetAllAssetsByKeyWord in Asset Store/AssetStore/Controllers/AssetController.cs, specifically where joinedKeyWords is constructed. We will sanitize each keyword via e.g. Replace("\r", "").Replace("\n", ""). No new dependencies are required.
| @@ -2,6 +2,7 @@ | ||
| using AssetStore.Services.Asset; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using SharpEngine.Shared.Dto.AssetStore; | ||
| using System.Linq; | ||
|
|
||
| namespace AssetStore.Controllers | ||
| { | ||
| @@ -44,7 +45,8 @@ | ||
| [HttpGet("/keyword")] | ||
| public ActionResult<IEnumerable<Asset>> GetAllAssetsByKeyWord([FromBody] IReadOnlyList<string> keywords) | ||
| { | ||
| var joinedKeyWords = string.Join(", ", keywords); | ||
| var sanitizedKeywords = keywords.Select(k => k?.Replace("\r", "").Replace("\n", "") ?? ""); | ||
| var joinedKeyWords = string.Join(", ", sanitizedKeywords); | ||
| _logger.LogDebug("Retrieving assets for the given keywords: {KeyWords}.", joinedKeyWords); | ||
|
|
||
| var assets = _service.GetAssets(keywords); |
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
The best way to fix this problem is to sanitize asset.Name to remove any characters that could forge log lines, especially line breaks (\r and \n). This can be done by replacing them with empty strings before logging. The fix should be applied directly in the logging call at line 83 (and elsewhere, if relevant). Use string.Replace to remove both Environment.NewLine, \r, and \n for completeness. Only the log statements that include user input need to be fixed. We should make similar changes to log calls in other endpoints that use user-provided strings if any (e.g., line 86, 98 in this file), but CodeQL has flagged only line 83 so we focus on this one. If asset.Name could be null, handle that gracefully.
| @@ -80,7 +80,8 @@ | ||
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var safeAssetName = asset.Name?.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", safeAssetName, asset.AuthorId); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the issue, ensure that any user input written to logs is sanitized. In this case, add a sanitization step when logging asset.Name and asset.AuthorId, making sure that if these properties are strings or can be stringified, any new line (or problematic) characters are removed prior to logging. This typically involves replacing or removing newline (\n, \r) characters and making sure they are suitable for display in a single log entry. Since the rest of the code logs multiple fields from asset, similar care should be taken wherever values derived from user input are logged. The minimal, non-intrusive fix is to use .ToString().Replace("\r", "").Replace("\n", "") on asset.AuthorId (and optionally on asset.Name) in the logger call at line 83 in CreateAsset. No new using directives are needed, as string.Replace is always available.
| @@ -80,7 +80,11 @@ | ||
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| _logger.LogDebug( | ||
| "Creating asset '{Asset}'. Published by '{Author}'.", | ||
| asset.Name?.Replace("\r", "").Replace("\n", ""), | ||
| asset.AuthorId.ToString().Replace("\r", "").Replace("\n", "") | ||
| ); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix this issue, we should sanitize asset.Name before including it in the log entry. For plain text logs (the most common case), this means we should remove or replace newline (\n, \r) characters so that a user cannot forge or manipulate the structure of the log file. The recommended approach is to use string.Replace() (for both Environment.NewLine and \n/\r) on asset.Name before logging it, ideally replacing them with an empty string or a visible placeholder. We specifically need to change the logging statements that interpolate asset.Name, including the one on line 86 and any similar log entries in the snippet where user input flows to log (e.g., line 83 and line 98). We will only modify the code shown.
This involves modifying how asset.Name is used in logging:
- Introduce a helper method or a local variable to sanitize the name.
- Replace occurrences of
asset.Namein_logger.LogDebugwith the sanitized value.
| @@ -80,10 +80,10 @@ | ||
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var sanitizedAssetName = asset.Name?.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", sanitizedAssetName, asset.AuthorId); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", sanitizedAssetName, asset.AuthorId); | ||
| return result; | ||
| } | ||
|
|
||
| @@ -95,7 +94,8 @@ | ||
| // TODO: Replace existing with revised asset | ||
| var result = _service.ReviseAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var sanitizedAssetName = asset.Name?.Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", sanitizedAssetName, asset.AuthorId); | ||
| return asset; | ||
| } | ||
|
|
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the log forgery risk, sanitize any user-provided value before logging it. Since asset.AuthorId is a user-provided value, before including it in any log entry, ensure it does not contain line breaks or other log-forging characters. For a Guid, the .ToString() method should only render valid Guid format, but additional caution is warranted in case user input is injected through model manipulation or a non-standard serializer. For robustness, use the .ToString() representation and remove any newlines before logging. The same applies to asset.Name if it is also user-controlled and included in logs.
Therefore, directly before logging on line 86 (and similarly for line 83 and 98 if desired), sanitize any string arguments that originate from user input, especially asset.AuthorId and asset.Name. For Guid types, ensure .ToString() is used and strip any newlines. For string properties, strip/replace newlines and escape as needed.
No new external libraries are required.
| @@ -80,10 +80,13 @@ | ||
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| // Sanitize asset.Name and asset.AuthorId before logging | ||
| var sanitizedName = asset.Name?.Replace("\r", "").Replace("\n", ""); | ||
| var sanitizedAuthorId = asset.AuthorId.ToString().Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", sanitizedName, sanitizedAuthorId); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", sanitizedName, sanitizedAuthorId); | ||
| return result; | ||
| } | ||
|
|
||
| @@ -95,7 +96,9 @@ | ||
| // TODO: Replace existing with revised asset | ||
| var result = _service.ReviseAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var sanitizedName = asset.Name?.Replace("\r", "").Replace("\n", ""); | ||
| var sanitizedAuthorId = asset.AuthorId.ToString().Replace("\r", "").Replace("\n", ""); | ||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", sanitizedName, sanitizedAuthorId); | ||
| return asset; | ||
| } | ||
|
|
| // TODO: Replace existing with revised asset | ||
| var result = _service.ReviseAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
The recommended way to fix this issue is to sanitize the user input before logging it. Since the log files are likely plain text (typical for backend logs), we should remove line breaks (\r, \n) from asset.Name before logging, using String.Replace. We should apply this sanitization in all log statements that include asset.Name. Specifically, lines 83, 86, and 98 include asset.Name in logs and should be fixed. The change can be local: create a sanitized copy of asset.Name in each method before logging, and use it in the log statements. No new methods or imports are needed.
| @@ -80,10 +80,11 @@ | ||
| return ReviseAsset(asset); | ||
| } | ||
|
|
||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var sanitizedName = asset.Name?.Replace("\r", "").Replace("\n", ""); // sanitize user input | ||
| _logger.LogDebug("Creating asset '{Asset}'. Published by '{Author}'.", sanitizedName, asset.AuthorId); | ||
| var result = await _service.CreateAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| _logger.LogDebug("Successfully created asset '{Asset}'. Published by '{Author}'.", sanitizedName, asset.AuthorId); | ||
| return result; | ||
| } | ||
|
|
||
| @@ -95,7 +94,8 @@ | ||
| // TODO: Replace existing with revised asset | ||
| var result = _service.ReviseAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); | ||
| var sanitizedName = asset.Name?.Replace("\r", "").Replace("\n", ""); // sanitize user input | ||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", sanitizedName, asset.AuthorId); | ||
| return asset; | ||
| } | ||
|
|
| // TODO: Replace existing with revised asset | ||
| var result = _service.ReviseAsset(asset); | ||
|
|
||
| _logger.LogDebug("Successfully revised asset '{Asset}'. Published by '{Author}'.", asset.Name, asset.AuthorId); |
Check failure
Code scanning / CodeQL
Log entries created from user input High




#52 Add Asset Store and
Contents
This PR is trying to resolve:
Store gamification information in a centralized user database.
Let users create a community to provide ready made assets.
We resolve it by:
Creating a template for the two projects and get them into git. (WIP)
Checklist
mainto my branch.