# WolvenKit code analysis: things to know before modding Cyberpunk 2077

We're all keen on playing games, but some of us also enjoy creating various mods\. Today, let's take a peek at WolvenKit, one of the modding tools for Cyberpunk 2077\.

![1165_WolvenKit/image1.png](https://import.viva64.com/docx/blog/1165_WolvenKit/image1.png)

## Intro

I think most of us have installed mods in the closest\-to\-our\-heart games\. And there are plenty of reasons for that: we want to add some new gameplay mechanics and skins, enhance the graphics, and just bring more hilarity to the game\. For instance, that's what I did in The Elder Scrolls V: Skyrim\. And I guess I'm not the only one\. I mean, this is exactly the sort of game where players could transform dragons into Thomas the Tank Engine, and Dovahkiin could fight this creature\.

![1165_WolvenKit/image2.gif](https://import.viva64.com/docx/blog/1165_WolvenKit/image2.gif)

Some even created their own mods for the games\. Today, we're going to check the project for game modding and see how its code is set up\. Let's take a look at the errors and suspicious code fragments that the static analyzer has detected\. I found this project on GitHub\. [WolvenKit](https://github.com/WolvenKit/WolvenKit) is an open\-source modding tool for Cyberpunk 2077 written in C\#\.

![1165_WolvenKit/image3.png](https://import.viva64.com/docx/blog/1165_WolvenKit/image3.png)

We use the PVS\-Studio 7\.32 static code analyzer\. If you're interested, you can get a free trial key on this [page](https://pvs-studio.com/en/pvs-studio/try-free/) and evaluate the latest analyzer version\.

Ready? Steady? Go\!

## Errors and stranger things

**Issue 1**

As with any program, we need to install WolvenKit before using it\. [WolvenKit\.Installer](https://github.com/WolvenKit/WolvenKit.Installer) is also available on GitHub\. It's also written in C\#\. I've decided not to leave it aside and analyze it\. There's not much code, but the analyzer still managed to detect a bug\.

```cpp
public async Task<bool> InstallAsync(....)
{
  ....
  try
  {
    ....
  }
  catch (Octokit.ApiException)
  {
    var apiInfo = ghClient.GetLastApiInfo();
    var rateLimit = apiInfo?.RateLimit;
    var howManyRequestsCanIMakePerHour = rateLimit?.Limit;
    var howManyRequestsDoIHaveLeft = rateLimit?.Remaining;
    var whenDoesTheLimitReset = rateLimit?.Reset;

     _logger.LogInformation(
    $"[Update] {howManyRequestsDoIHaveLeft}/{howManyRequestsCanIMakePerHour}" +
    $" - reset: {whenDoesTheLimitReset 
    ?? whenDoesTheLimitReset.Value.ToLocalTime()}");
    return false;
  }
}
```

The PVS\-Studio warning: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'whenDoesTheLimitReset' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. LibraryService\.cs 332

The error lurks in the exception handler :\) The analyzer reports that developers use the _whenDoesTheLimitReset_ variable, although it gets its value using the '?\.' operator\. This could mean that it may be equal to _null_\. And we see a check for _null_ using the '??' operator\. However, the variable will be really used if its value is _null_\. As a result, we get a _NullReferenceException_\. Hmm, what caused the error? Developers might not know how the '??' operator works, or they just didn't spot it in the code\.

**Issue 2**

Let's look at the bugs in the main project\.

```cpp
public AppImpl()
{
  ....
  // load oodle
  var settingsManager = Locator.Current.GetService<ISettingsManager>();
  if (   settingsManager.IsHealthy() 
      && !Oodle.Load(settingsManager?.GetRED4OodleDll()))
  {
    throw new FileNotFoundException($"{Core.Constants.Oodle} not found.");
  }
}
```

The PVS\-Studio warning: [V3042](https://pvs-studio.com/en/docs/warnings/v3042/) Possible NullReferenceException\. The '?\.' and '\.' operators are used for accessing members of the 'settingsManager' object App\.xaml\.cs 48

Here's another _null_ error\. Developers used the _settingsManager_ variable twice in the condition of the _if_ statement\. They initially used it without _null_ checking, and then with the check via '?\.'\. We might assume that _IsHealthy_ is an extension method, so the exception won't be thrown\. Unfortunately, it's a regular instance method\.

**Issue 3**

```cpp
public EFileReadErrorCodes ReadBuffer(RedBuffer buffer)
{
  ....
  data.Uk1 = _reader.ReadUInt32();
  data.Uk2 = _reader.ReadUInt32();
  data.Uk3 = _reader.ReadUInt32();
  var numBrck = _reader.ReadUInt32();
  var numSurf = _reader.ReadUInt32();
  var numProb = _reader.ReadUInt32();
  var numFact = _reader.ReadUInt32();
  var numTetr = _reader.ReadUInt32();
  data.Uk4 = _reader.ReadUInt32();
  data.Uk5 = _reader.ReadUInt32();
  data.Uk5 = _reader.ReadUInt32();      // <=

  data.Bounds.Min.X = _reader.ReadSingle();
  ....
}
```

The PVS\-Studio warning: [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) The 'data\.Uk5' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 35, 34\. CGIDataReader\.cs 35

Here's an example of a common error\. The _data\.Uk5_ property is assigned a value twice\. Developers might have copied a line, change the digit, but then forgotten about it in the last line\. It's hardly a redundant line, since there's the _data\.Uk6_ property\.

**Issue 4**

```cpp
public void Load()
{
  ....
  for (....; sectorIndex < pgc.BufferTableSectors.Count; ....)  // <=
  {
    if (   pgc.SectorEntries == null 
        || pgc.SectorEntries.Count <= sectorIndex 
        || pgc.SectorEntries[sectorIndex] == null)
    {
      throw new ArgumentNullException();
    }

    var sectorHash = pgc.SectorEntries[sectorIndex]!.SectorHash;
    if (!_entries.ContainsKey(sectorHash))
    {
      _entries[sectorHash] = new();
    }

    if (   pgc.BufferTableSectors == null                    // <=
        || pgc.BufferTableSectors.Count <= sectorIndex 
        || pgc.BufferTableSectors[sectorIndex] == null)
    {
      throw new ArgumentNullException();
    }
  }
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'pgc\.BufferTableSectors' object was used before it was verified against null\. Check lines: 43, 56\. GeometryCacheService\.cs 43

The analyzer has detected that developers used the _pgc\.BufferTableSectors_ property before checking it for _null_\.

**Issue 5**

```cpp
private void LoadInfo()
{
  if (_projectManager.ActiveProject is null)
  {
    return;
  }

  var path = Path.Combine(Environment.CurrentDirectory,
                          "Resources",
                          "soundEvents.json");
  path = Path.Combine(_projectManager.ActiveProject.ResourcesDirectory,
                      "info.json");
    if (File.Exists(path))
    {
      ....
    }
}
```

The PVS\-Studio warning: [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) The 'path' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 82, 81\. SoundModdingViewModel\.cs 82

It's probably not an error but a suspicious code fragment\. The _path_ variable is assigned a value twice\. Developers might have left one of these values after debugging, or it might just be the development artifact\.

**Issue 6**

```cpp
private bool InsertArrayItem(IRedArray ira, int index, IRedType item)
{
  var iraType = ira.GetType();
  if (iraType.IsGenericType)
  {
    var arrayType = iraType.GetGenericTypeDefinition();
    if (   arrayType == typeof(CArray<>) 
        || arrayType == typeof(CStatic<>) 
        && ira.Count < ira.MaxSize)
    {
      ....
    }
  }
}
```

The PVS\-Studio warning: [V3130](https://pvs-studio.com/en/docs/warnings/v3130/) Priority of the '&&' operator is higher than that of the '\|\|' operator\. Possible missing parentheses\. ChunkViewModel\.cs 3053

There's another suspicious code fragment\. It's tough to say for sure if it's a bug or not, but it's worth taking a peek at this code\. The analyzer warns about a possible issue related to the operator precedence\. As we all know, the '&&' operator takes precedence over '\|\|'\. It means that the _ira\.Count < ira\.MaxSize_ expression is evaluated only with _arrayType \=\= typeof\(CStatic<\>\)_, not with the entire OR expression\. If developers wanted it, it'd be a good idea to put the AND expression in parentheses for readability\.

**Issue 7**

```cpp
private void AddCurrent(worldNodeData current)
{
  ....
  if (Parent?.Data is DataBuffer db && db.Buffer.Data is IRedType irt)
  {
    if (irt is IRedArray ira && ira.InnerType.IsAssignableTo(current.GetType()))
    {
      var indexx = Parent.GetIndexOf(this) + 1;
      if (indexx == -1 || indexx > ira.Count)      // <=
      {
        indexx = ira.Count;
      }
      ira.Insert(indexx, current);
    }
  }
}
```

The PVS\-Studio warning: [V3063](https://pvs-studio.com/en/docs/warnings/v3063/) A part of conditional expression is always false if it is evaluated: indexx \=\= \-1\. ChunkViewModel\.cs 4201

The analyzer has detected that the _indexx \=\= \-1_ expression is always _false_\. Why does the analyzer warn about this? Is it correct? Let's dive in\. The _indexx_ variable gets its value from the _Parent\.GetIndexOf\(this\) \+ 1_ expression\. Look at the _GetIndexOf_ method\.

```cpp
public int GetIndexOf(ChunkViewModel child)
{
  if (child.NodeIdxInParent > -1)
  {
    return child.NodeIdxInParent;
  }
  
  for (var i = 0; i < Properties.Count; i++)
  {
    if (ReferenceEquals(Properties[i], child))
    {
      child.NodeIdxInParent = i;
      return i;
    }
  }
  return -1;
}
```

As we can see, the minimum return value from the method is \-1\. In the expression, developers added 1 to the return method value\. So, the analyzer is right\. Developers either probably didn't need to add 1, or they should've tweaked the condition\.

The analyzer traverses the code and tracks possible values for variables and constraints\. This mechanism is called [**Data Flow**](https://pvs-studio.com/en/blog/terms/7004/)\. This is how the analyzer spots these kinds of errors\.

**Issue 8**

```cpp
public static Dictionary<string, string> GetPropertiesFor(....)
{
  ....
  foreach (var appearance in appearancesArr)
  {
    details[....] = appearance.AppearanceName.ToString()!;
    details[....] = appearance.IsPlayer == true ? "True" : "False";
    details[....] = ParseGameEntityReference(appearance?.PuppetRef);

    counter++;
  }
  ....
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'appearance' object was used before it was verified against null\. Check lines: 547, 548\. NodeProperties\.cs 547

Here's a simple error: three lines use the _appearance_ variable\. In the last line, developers used _appearance_ with the '?\.' operator\. So, the variable may be _null_\.

**Issue 9**

```cpp
public int UncookTask(FileSystemInfo[] paths, UncookTaskOptions options)
{
  ....
  if (options.outpath == null)       // <=
  {
    _loggerService.Error("Please fill in an output path.");
    return ERROR_BAD_ARGUMENTS;
  }

  if (   options.meshExportType != null
      && string.IsNullOrEmpty(options.meshExportMaterialRepo) 
      && options.outpath is null)     // <=
  {
    _loggerService.Error("When using --mesh-export-type, the --outpath" +
    " or the --mesh-export-material-repo must be specified.");
    return ERROR_INVALID_COMMAND_LINE;
  }
}
```

The PVS\-Studio warning: [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) Expression 'options\.meshExportType \!\= null && string\.IsNullOrEmpty\(options\.meshExportMaterialRepo\) && options\.outpath is null' is always false\. UncookTask\.cs 44

The analyzer warns that the large expression is always _false_\. And that's really true\. All the subexpressions are connected via '&&', and the last one has already been in the previous _if_ statement, followed by the method exit\.

**Issue 10**

```cpp
private static string TryGetGameInstallDir()
{
  if (   programName?.ToString() is string n 
      && installLocation?.ToString() is string l)
  {
    if (n.Contains(gameName) || n.Contains(gameName))  // <= 
    {
      exePath = Directory.GetFiles(l, exeName, SearchOption.AllDirectories)
                         .First();
    }
  }
}
```

The PVS\-Studio warning: [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'n\.Contains\(gameName\)' to the left and to the right of the '\|\|' operator\. Oodle\.cs 488

The analyzer has detected the subexpressions that are identical to the ones on either side of the '\|\|' operator\. Developers may need to use the _l_ variable in the second subexpression\.

**Issue 11**

```cpp
public void Extract(Stream output, bool decompressBuffers)
{
  if (Archive is not { } ar)
  {
    throw new InvalidParsingException(
      $"{Archive.ArchiveAbsolutePath} is not a cyberpunk77 archive.");
  }

  ar.CopyFileToStream(output, NameHash64, decompressBuffers);
}
```

The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'Archive'\. FileEntry\.cs 87

Essentially, _Archive is not \{ \}_ is the same as _Archive is not object_ or _Archive is null_\. If the check is _true_, developers get a _NullReferenceExceprion_ when an error exception is thrown\. 

**Issue 12**

```cpp
public ButtonCursorStateView()
{
  HoverStateName = "Hover";
  PressStateName = "Hover";
  DefaultStateName = "Default";

  PostConstruct();
}
```

The PVS\-Studio warning: [V3091](https://pvs-studio.com/en/docs/warnings/v3091/) Empirical analysis\. It is possible that a typo is present inside the string literal: "Hover"\. The 'Hover' word is suspicious\. ButtonCursorStateView\.cs 34

Let's bring in some heuristic analysis\. It seems that the analyzer has detected the failed copy\-paste, and developers should assign_ "Press"_ to _PressStateName_\.

**Issue 13**

```cpp
public static int GetCompressedBufferSizeNeeded(int count)
{
  var n = (((count + 0x3ffff + ((uint)((count + 0x3ffff) >> 0x3f) & 0x3ffff))
    >> 0x12) * 0x112) + count;
  //var n  = OodleNative.GetCompressedBufferSizeNeeded((long)count);
  return (int)n;
}
```

The PVS\-Studio warning: [V3134](https://pvs-studio.com/en/docs/warnings/v3134/) Shift by 63 bits is greater than the size of 'Int32' type of expression '\(count \+ 0x3ffff\)'\. Oodle\.cs 397

And here's the last suspicious fragment\. Developers wanted to shift to _0x3f_, which is equal to 63\. However, it exceeds the size of the expression type to which the shift operation is applied\. This expression is of the _Int32_ type, which contains 32 bits\. Since shifts are circular in C\#, the shift of 63 is equal to that of 31\. 

## Outro

We've covered all the bugs and suspicious code fragments in the WolvenKit project\! While I haven't listed every single bug in this article due to some being repetitive, I'll be reporting all of them directly to the developers via the Issue on GitHub\.

Stay tuned for updates and happy developing\!