[Bugfix] Updating scorehash on every password changes/Explicitly save config.xml into shift_jis by Unengine · Pull Request #6 · Unengine/LR2Nexus

Unengine · 2026-06-28T01:44:16Z

Changes

Fix config.xml to be saved explicitly in shift_jis and xml version 1.0
Make every scorehash in scores table in <player>.db updated when the password is changed.

Related Issues

chown2 · 2026-06-28T10:59:14Z

+				updateScoreCmd.Parameters.Add("@hash", SqliteType.Text);
+				updateScoreCmd.Prepare();
+
+				using (var selectScoresCmd = new SqliteCommand(SelectScoresQuery, connection, transaction))


I'd make sure current 'scorehash' values are valid, otherwise 'scorehash' as a concept loses value
You can even do it in the same loop since you have a transaction, just don't forget to ROLLBACK on bad scores.
To check scorehash validity, you'd have to add some fallbacks that manipulate 'rank'.
Use OpenLR2 as a reference: https://github.com/GOMazk/OpenLR2/blob/15b2fc066a8bdbfcc28d3d2a88e6467048f7bd7f/LR2/LR2_statlong.cpp#L190

I’ve added the validation logic as you suggested. Any invalid scorehash is now automatically corrected to the valid one within the transaction loop. By ensuring only valid data is written, we prevent bad scores from being persisted, which keeps the data integrity solid without needing extra rollback steps.

Regarding the ROLLBACK strategy: I've ensured that any unexpected exception during the hash correction will trigger a rollback.

chown2 · 2026-06-28T11:04:01Z

+
+public class DBPlayerRow(SqliteDataReader reader)
+{
+	public string? Id { get; set; } = DatabaseUtils.GetValue<string>(reader, "id");


Here and everywhere below, is it necessary to have strings as optionals? I'm not familiar with C#

The goal is to explicitly distinguish between an intentional null assignment and an implicit default null state. While the type distinction might seem like extra overhead at first, it's far more cost-effective to handle these cases as compile-time warnings than to deal with runtime errors.

That's why C# strictly flags potential nullability for all variables at compile time. It acts as a contract: it either warns you that a variable might be null and requires handling, or gives you a strong guarantee that it won't be null, allowing you to use it safely. I'm leveraging these features to make the code's intent clearer and more robust.

In short, cleaning up all those compiler warnings by using nullable types gives me a sense of psychological safety. And honestly, that kind of peace of mind is invaluable XD

I know SQL tables don't have 'NOT NULL' in them, but they will never be NULL in practice.
If we detect a NULL, we should reject such database early.
If I understand you correctly, this nullability flag forces you to check that the string is not null every time you use it.
But it should be done just once when we read from the database.
Also, why are only string fields nullable, but not the int ones?

I know SQL tables don't have 'NOT NULL' in them, but they will never be NULL in practice.
If we detect a NULL, we should reject such database early.
If I understand you correctly, this nullability flag forces you to check that the string is not null every time you use it.
But it should be done just once when we read from the database.

SqliteDataReader.GetValue returns object.
If the returned value is casted to ref type, it is potentially nullable so we should declare it nullable.

Also, why are only string fields nullable, but not the int ones?

Cause string is originally ref type and int is value type. It is kinda hard to explain....

chown2 · 2026-06-28T11:08:42Z

+{
+	public static class MD5Util
+	{
+		public static string CalculateMD5(object[] dataRaw)


IMO, it would be more clear, if you concatenated all fields on the outside and then used 'CalculateMD5(string input)'.
Performance wise, this method already does 'string.Concat' anyway, so I have doubts it is any faster than 'CalculateMD5(string input)'.

chown2 · 2026-06-28T16:57:15Z

+			}
+			catch (OperationCanceledException)
+			{
+				Console.WriteLine("Password update cancelled.");


chown2 · 2026-06-28T17:01:48Z

+						if (!DBScoreRow.IsScorehashValid(score, targetPlayerPasswordHashMD5, targetScorehashMD5))
+						{
+							updateScoreCmd.Parameters["@scorehash"].Value = targetScorehashMD5.Body;
+							updateScoreCmd.Parameters["@hash"].Value = score.Hash;
+							updateScoreCmd.ExecuteNonQuery();
+							Console.WriteLine($"Updating scorehash of a score.\n[Update] Hash : {score.Hash} / Scorehash : {targetScorehashMD5}");
+						}


If an invalid scorehash is found, the database should be rejected
(pseudo-code, not sure if this compiles)

Suggested change

if (!DBScoreRow.IsScorehashValid(score, targetPlayerPasswordHashMD5, targetScorehashMD5))

{

updateScoreCmd.Parameters["@scorehash"].Value = targetScorehashMD5.Body;

updateScoreCmd.Parameters["@hash"].Value = score.Hash;

updateScoreCmd.ExecuteNonQuery();

Console.WriteLine($"Updating scorehash of a score.\n[Update] Hash : {score.Hash} / Scorehash : {targetScorehashMD5}");

}

if (!DBScoreRow.IsScorehashValid(score, targetPlayerPasswordHashMD5, targetScorehashMD5))

throw new Exception("Invalid score found in the database");

updateScoreCmd.Parameters["@scorehash"].Value = targetScorehashMD5.Body;

updateScoreCmd.Parameters["@hash"].Value = score.Hash;

updateScoreCmd.ExecuteNonQuery();

Console.WriteLine($"Updating scorehash of a score.\n[Update] Hash : {score.Hash} / Scorehash : {targetScorehashMD5}");

Anyone can theoretically generate a valid scorehash by reverse-engineering the algorithm, so scorehash's value as a security measure is definitely limited. I agree that it’s not a perfect defense against malicious users.

However, my intent is to ensure that the system remains consistent for all users. By force-updating the scorehash whenever it's invalid, I'm making sure that legitimate player data is always in a stable state and doesn't suffer from 'stale' or inconsistent records. I see this as maintaining data integrity for the game's overall reliability, rather than trying to block sophisticated attackers.

by reverse-engineering the algorithm

Exactly, and that's something a bit deeper than what a script kiddie could achive. But modifying a score with DB Browser for SQLite is very simple and would take just a few minutes even for someone who's never heard of SQL before.

doesn't suffer from 'stale' or inconsistent records

In LR2 itself, OpenLR2 included, there is no possiblity of that happening

In LR2 itself, OpenLR2 included, there is no possiblity of that happening

Yes, I definitely trust it!
But.... actually I don't trust LR2Nexus. Password can be changed with that, and it could be risky. And I want to handle that.

In other words, LR2Nexus might be a 'sophisticated attacker' with that.

If you don't trust LR2Nexus, how about making a backup of score DB file if the password is being changed?

Cool! I'm on my way.

Alright, I guaranteed the backup. If creating backup failed, I made it immediately throw exception to prevent further operations progressed.
I really appreciate your dedication and time for the LR2 community. Thanks for the help!

Unengine added 2 commits June 28, 2026 05:24

Explicitly save config.xml in shift_jis

7babde5

Update scorehash on updating password

b6d036c

Unengine self-assigned this Jun 28, 2026

Unengine added the bug Something isn't working label Jun 28, 2026

Remove useless usings

81531ca

chown2 reviewed Jun 28, 2026

View reviewed changes

Add scorehash validation

e495e7c

Unengine requested a review from chown2 June 28, 2026 16:42

Unengine added 2 commits June 29, 2026 01:45

Remove useless usings

c9dbbdf

Rename method name for better understanding

91f4064

chown2 reviewed Jun 28, 2026

View reviewed changes

Add tx rollback on OperationCanceledException

1f8c43c

Unengine requested a review from chown2 June 28, 2026 17:35

Unengine added 2 commits June 29, 2026 03:44

Create backup when password is changed

44b8dbd

Remove useless usings

e9dcf72

Unengine merged commit 5369b02 into main Jun 28, 2026
1 check passed

Unengine deleted the bugfix/xml-encoding-password-nuke branch June 28, 2026 18:56

This was referenced Jun 28, 2026

Changing password makes a player's scores invalid. #5

Closed

config.xml should be saved explicitly in CP932 or shift_jis and xml version 1.0 #4

Closed

Conversation

Unengine commented Jun 28, 2026

Changes

Related Issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unengine Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chown2 Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unengine Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unengine Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Unengine Jun 28, 2026 •

edited

Loading

chown2 Jun 28, 2026 •

edited

Loading

Unengine Jun 28, 2026 •

edited

Loading

Unengine Jun 28, 2026 •

edited

Loading