# Nephalem's nightmare\. Exploring errors in Diablo 3 server emulator code
Pursuing Diablo, the nephalem falls into another one of Belial's traps\. Plunged into a deadly nightmare by the Lord of Lies, the hero must repeat their past victories over the Great Evils to awaken\. This time, however, the nephalem is doomed to defeat due to the inexplicable distortions of reality inherent in nightmares\. But suddenly, a mysterious creature comes to help, and with it, hope arises once again\.
## Introduction
"There are a lot of things you can find on GitHub\!", I think as I browse it and always find some interesting and unusual projects\. This time, I came across a Diablo 3 game server [emulator](https://github.com/blizzless/blizzless-diiis) written in C\#\. Diablo 3 is a renowned game developed and published by [Blizzard Entertainment](https://www.blizzard.com/en-us/)\. Like the other games in the franchise, it has won quite a lot of fans\.
Being a developer in a company producing a static code analyzer, I couldn't resist testing our tool on the source code of one of my favourite games, even though it's not the original code\. As a result, I've found a couple of code fragments with both potential and quite obvious errors\. In this article, I suggest to take a look at them\.
To make the content more engaging and varied, I've covered some of the bugs in a Diablo\-themed narrative :\) We're going to take on the role of a mysterious being who helps the protagonist escape the nightmarish vision Belial has cast in their mind\. The story takes place many years after the defeat of Malthael, the Angel of Death, and the release of the Great Evils\. Some of them, including Belial, the Lord of Lies, have since regained much of their former power\.
Note that the connections between the stories and possible errors in the given code fragments are mostly made up and have been added to make the article more interesting\. You may think of the code review subsections as mini\-games, like the ones we go through when picking locks, cracking ciphers, or disarming traps in video games :\) However, the code fragments that contain potential errors are real\. They're present in the emulator source code at the time of writing the article\.
## Act I\. Caldeum
Trapped in a nightmare, the nephalem found themselves in the heart of Caldeum, a city known as the "Jewel of the East"\.
The Palace of Caldeum is where the hero once defeated Belial\. The task seemed obvious: infiltrate the palace and repeat the previous victory over the Lord of Lies\.
Battling the minions of the Lesser Evil, the nephalem wandered the city streets, searching in vain for a way to the palace\. In the nightmare, the city looked like a vast, unfamiliar maze of alleyways\. So, an hour passed, and then another, and another\.\.\. Eventually, the hero stumbled upon a folded parchment — a map of the city\. They unfolded the parchment\. It was\.\.\. blank\. "What a mockery\!", the nephalem thought as a strange creature suddenly appeared out of the bright light next to the hero\. It looked like something between a human and a unicorn \(you can meet all sorts of creatures in a nightmare, can't you?\), but the light it emitted made it difficult to see any particular detail\. The creature looked as if it didn't want to harm the hero\. On the contrary, it wanted to help\. As it approached the protagonist, the creature used a mysterious artefact \(let's call it a "distortion detector"\) next to the map\.
Let's dispel distortion
No matter how realistic an illusion is, there are always distortions in it — errors in its source code\. The absence of an image on the map seems to be the result of such distortion\.
```cpp
public MapRevealSceneMessage MapRevealMessage(Player plr)
{
if (
.... ||
SceneSNO.Id == 183556 ||
SceneSNO.Id == 183557 ||
SceneSNO.Id == 183502 ||
SceneSNO.Id == 183505 ||
SceneSNO.Id == 183557 ||
SceneSNO.Id == 183519 ||
....
)
{
return new MapRevealSceneMessage
{
....
MiniMapVisibility = true
};
}
else
{
return new MapRevealSceneMessage
{
....
MiniMapVisibility = false
};
}
}
```
To dispel it and help the hero, try to find an error in the following code fragment, and then compare your answer with the one below\.
The answer is hidden here
The distortion detector message:
[V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'SceneSNO\.Id \=\= 183557' to the left and to the right of the '\|\|' operator\. Scene\.cs 581
In the above condition, the OR operator is probably used to enumerate the identifiers of the scenes \(location options\) in the city for which the mini\-map should be visible\. It would be reasonable to assume that all identifiers in the condition should be unique, but the distortion detector showed two identical identifiers \(constants\) with the 183557 value\. One of them could be accidentally written instead of the unique identifier that belongs to the other scene\. So, the _MiniMapVisibility_ property for this scene now is erroneously set to _false_\.
## Act II\. Burning Hells
After the creature dispelled the distortion, the image appeared on the map\. With it, the nephalem easily found their way to the palace, where they once again defeated Belial \(it was hardly the real Belial, but rather his nightmarish copy\)\.
Upon leaving the palace, the hero inexplicably — as it often happens in nightmares — shifted straight to Hell and found themselves completely immobilised for some reason\. The nephalem recognized the place where they ended up: here they had defeated Azmodan — another of the Great Evils\. As soon as the nephalem thought about him, the monstrous voice of the Lord of Sin thundered from somewhere behind, rushing towards the defenseless hero\.
In a moment of sheer desperation, the mysterious creature once again came to the rescue\. It was now clear that the hero's inability to move was a result of some distortions\.
The creature is trying to free the nephalem, but does it have enough time?
Let's dispel distortions
As you did last time, try to find an error in each of these code fragments\. Remember: the nephalem's life depends on your success\!
**Distortion 1**
```cpp
public override bool Reveal(....)
{
....
if (!randomed && Destination.DestLevelAreaSNO != 19794)
....
if (World.IsPvP && Destination != null && ....)
....
if (Destination != null)
....
if (Destination == null || ....)
....
}
```
The answer is hidden here
The distortion detector message:
[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'Destination' object was used before it was verified against null\. Check lines: 1241, 1244\. Portal\.cs 1241
The detector indicates that the _Destination_ property in the first conditional statement could potentially have a value of _null_, as indicated by all subsequent checks of the property\. If _Destination_ is _null_, an exception is thrown when _Destination\.DestLevelAreaSNO_ is dereferenced, causing the whole program to stop running\.
To fix a potential issue, let's add an appropriate check to the first conditional statement:
```cpp
if (!randomed &&
Destination != null &&
Destination.DestLevelAreaSNO != 19794)
```
**Distortion 2**
```cpp
public override void Encode(GameBitBuffer buffer)
{
....
buffer.WriteBool(EngagedWithRareTime.HasValue);
if (EngagedWithRareTime.HasValue)
{
buffer.WriteInt(32, EngagedWithGoblinTime.Value);
}
buffer.WriteBool(EngagedWithGoblinTime.HasValue);
if (EngagedWithGoblinTime.HasValue)
{
buffer.WriteInt(32, EngagedWithGoblinTime.Value);
}
buffer.WriteBool(InCombat.HasValue);
if (InCombat.HasValue)
{
buffer.WriteBool(InCombat.Value);
}
....
}
```
The answer is hidden here
The distortion detector message:
[V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'EngagedWithGoblinTime' object was used before it was verified against null\. Check lines: 190, 192\. TrickleMessage\.cs 190
The detector shows a similar issue here\. The value of the _EngagedWithGoblinTime\.HasValue_ property is written to the _buffer_, indicating that _EngagedWithGoblinTime_ may be _null_\. A few lines above, _EngagedWithGoblinTime\.Value_ is dereferenced, which may lead to an exception\.
If you look at the condition of the _if_ statement, in which the dangerous dereference occurs, you can easily see that it checks whether a property with a very similar name to _EngagedWithGoblinTime_ has a value\. One could think that instead of the _EngagedWithRareTime_ value, the _EngagedWithGoblinTime_ value should be checked in the condition\. However, if we look at the following conditional operator, we can see that _EngagedWithGoblinTime_ and _EngagedWithRareTime_ are mixed up, indeed: not in the condition of the first _if_, but in its _then_ block when dereferenced\. So, here's how we can fix it:
```cpp
if (EngagedWithRareTime.HasValue)
{
buffer.WriteInt(32, EngagedWithRareTime.Value);
}
buffer.WriteBool(EngagedWithGoblinTime.HasValue);
```
## Act III\. High Heavens
Just mere moments before Azmodan struck, the creature managed to free the hero from the immobility\. The nephalem instantly dodged the attack and, without wasting any time, violently struck back\. It was a tough fight\. Azmodan summoned his minions to help, but the nephalem still managed to win\.
Once the Lord of Sin had fallen, the environment around the hero began to rapidly change\. The scenery of the Burning Hells gave way to the scenery of\.\.\. the blazing Heaven\.
The nephalem stood at the entrance to the Crystal Arch — the source of all angels' power\. That's where the hero once defeated the most insidious of the Great Evils — Diablo — when he, having absorbed the powers of all the other Great Evils, attempted to desecrate the Arch and thus claim the final victory in the war against Heaven\.
It wasn't hard to guess what awaited the hero on the other side of the entrance, but there was no other choice\. After all, the nephalem had already won this fight once, they could handle it now\. With that in mind, the nephalem walked towards the Arch\.
Things went wrong within the first few minutes of the fight\. The protagonist's blows did no discernible damage to the Lord of Terror, but each of Diablo's attacks seemed deadly\. Even the heat emanating from him felt so strong, as if the hero wasn't wearing any armor at all\.
No doubt there was some distortion here\. But it didn't make Diablo completely invincible\. It changed the stats of the hero's own weapons and armor, making them the most mediocre\. It would be dangerous to use such equipment against a reanimated skeleton, let alone the Prime Evil\.
Clearly, the fight was hopeless\. The nephalem dodged the attacks as best as they could, but their luck wasn't infinite\. In the end, the hero missed a not\-so\-hard blow that could've been fatal\. They fell to the ground, and the Lord of Terror prepared to deliver the final blow\. At that moment, just in time as always, the nephalem's mysterious sidekick appeared to take the brunt of the blow\. Severely wounded, the creature flew to the side and crashed next to the protagonist\. With its last ounce of strength, it used the artefact, hoping to dispel the distortion in time\.
#### Let's dispel distortion
```cpp
public static void Generate(....)
{
....
filteredList = filteredList.Where(
a =>
!a.Name.Contains("FireD") &&
!a.Name.Contains("PoisonD") &&
!a.Name.Contains("HolyD") &&
!a.Name.Contains("ColdD") &&
!a.Name.Contains("LightningD") &&
!a.Name.Contains("ArcaneD") &&
!a.Name.Contains("MinMaxDam") &&
isCrafting ? !a.Name.ToLower().Contains(....)
: !a.Name.Contains(....)
);
....
}
```
The answer is hidden here
The distortion detector message:
[V3123](https://pvs-studio.com/en/docs/warnings/v3123/) Perhaps the '?:' operator works in a different way than it was expected\. Its priority is lower than priority of other operators in its condition\. AffixGenerator\.cs 207
Look at the left side of the ternary expression\. At first glance, it looks like this: "isCrafting ? \.\.\.\."\. However, first impressions can often be deceiving\. The precedence of the _?_ operator is lower than that of the _&&_ operator, so the left side of the ternary operator actually looks like this:
```cpp
!a.Name.Contains("FireD") &&
....
!a.Name.Contains("MinMaxDam") &&
isCrafting ? ....
```
It seems that the code author didn't consider this, so the conditional expression passed to _Where_ may not work as expected\. We can resolve the potential issue by adding parentheses around the ternary expression:
```cpp
filteredList = filteredList.Where(
a =>
....
!a.Name.Contains("MinMaxDam") &&
(isCrafting ? ....)
);
```
#### Epilogue
With a feeling of absolute superiority, Diablo struck again, finally killing the hero\.\.\. or so he thought\. Suddenly, the attack bounced off of the armor, leaving only a scratch on it\. The distortion dissipated, and the nephalem's equipment regained its former power\! The hero perked up, sprang up with a swift movement, and charged at the demon\. Just like that day, the Heavens were long shaken by the battle\. Just like that day, Diablo has fallen\. Belial's curse began to break\. The protagonist then turned their attention to their companion\. The hero wanted to go over and help him, to ask who he was, how he got here, and why he was helping them\.\.\. So many questions and so little time\. The nightmare dissipated, and the hero woke up\.
## But we're not finished yet\.\.\.
This little story is over\. However, in the previous section, I haven't covered all the code fragments containing errors that I'd like to discuss in this article\. I want to undo this injustice and cover them here\. I invite anyone who wants to dig a little deeper into the code to take a look\.
To level up your analytical skills, try to find one error in each code fragment by yourself and then check the answers\.
**Case 1**
```cpp
public static void GrantCriteria(....)
{
....
else
{
....
uint alcount = alreadycrits.CriteriaId32AndFlags8;
var newrecord = D3.Achievements
....
.SetQuantity32(alcount++) // <=
.Build();
int critCount = UnserializeBytes(achievement.Criteria).Count;
if (critCount >= 5)
{
GrantCriteria(client, 74987246353740);
}
client.Account.GameAccount.AchievementCriteria
.Remove(alreadycrits);
client.Account.GameAccount.AchievementCriteria
.Add(newrecord);
}
....
}
```
The answer is hidden here
The PVS\-Studio warning:
[V3159](https://pvs-studio.com/en/docs/warnings/v3159/) Modified value of the 'alcount' operand is not used after the postfix increment operation\. AchievementManager\.cs 360
We expect that _alcount_ should be incremented by 1, then the resulting value should be passed to the _SetQuantity32_ method\. In reality, the opposite happens: first, the _alcount_ value is passed to the method, and only then the value of the variable is incremented by 1\.
We can easily fix the error by replacing _alcount\+\+_ with the _alcount \+ 1_ or _\+\+alcount_ expression\.
**Case 2**
```cpp
public class LooterBrain : Brain
{
....
public override void Think(int tickCounter)
{
....
if (LootLegendaries)
targets.Concat(....);
....
}
....
}
```
The answer is hidden here
The PVS\-Studio warning:
[V3010](https://pvs-studio.com/en/docs/warnings/v3010/) The return value of function 'Concat' is required to be utilized\. LooterBrain\.cs 154
Even if _LootLegendaries_ is _true_, no new objects are added to _targets_\. That happens because the _Concat_ method doesn't modify any of the original enumerations but creates a new one\. To fix the error, we can assign the result of the _Concat_ method execution to the _targets_ variable\.
In the game, the error lies in in the inability of loot\-collecting pets to pick up legendary items\. This is indicated by the context in which the _LooterBrain_ class is used, with the _LootLegendaries_ property set to _true_\. Its value is set using the second constructor argument:
```cpp
public LooterPetAnniversary(....): base(....)
{
....
SetBrain(new LooterBrain(this, true));
....
}
```
**Case 3**
```cpp
public override bool Reveal(....)
{
bool isGolem = false;
if (this is BaseGolem ||
this is IceGolem ||
this is BoneGolem ||
this is DecayGolem ||
this is ConsumeFleshGolem ||
this is BloodGolem)
{
....
isGolem = false;
}
....
if (....)
player.InGameClient.SendMessage(new PetMessage()
{
....
Index = isGolem ? 9 : player.CountFollowers(SNO) + PlusIndex,
....
});
}
```
The answer is hidden here
The PVS\-Studio warning:
[V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'isGolem' is always false\. Minion\.cs 230
It seems reasonable to assume that within the first conditional expression, the _isGolem_ variable should be assigned the _true_ value, but instead it is reassigned _false_\. As a result, an incorrect value may be assigned to the _Index_ property\.
## Conclusion
I hope this article has refreshed your memories of past adventures in Diablo 3 or, if you haven't played it yet, made you want to\. Also, this article has given you the opportunity to practice in analyzing C\# code\. It's no secret that learning from other people's mistakes minimises the risk of making them in your own code\.
By the way, if you're interested in the tool used to find the errors, you can download it from the [official website](https://pvs-studio.com/en/pvs-studio/try-free/) and try it for free\.
See you in the next articles\!