What does your code smell?

- Well it doesn’t have a nose… but it definitely can stink!` (SourceMaking.com, 2007–2020).

If your code smells, hackers will come for you. This formula should be the firston your list to learn anything about programming. If any part of the code is leaking or rotting, the payday shall not scape. It is a litmus test for cyber crooks. They are of the same programming origin. They know that the code can smell, that it smells and the smell is deviating. It would not be much of surprise if black hat hackers had kind of the scale to evaluate the level of the smell.

It is clear as a day, that clean code is much easier to be detected for mistakes. It is interesting to notice that in the programming world it is much harder to write simple and generic features than those with complex logic. The more badly designed it is the cleaner and harder to write. “Clean code — is dumb but, very understandable code. And the more it is duller the more it is clear “. Eventually, it makes the programme more clever. Paradox. This is because the developers write a part of an application not only for themselves but they also commit to teamwork. If the code written with some pattern has not been agreed on within the team then bad architecture will produce difficulties for other developers to understand and will badly affect the efficiency. So they need to clean a lot after themselves. They were told from childhood by parents that those who have not been studying well will end up with a cleaning job. Makes a lot of sense now, right? At 3 a.m. while trying not to be sucked by the bugs attacking from the console. So, if you only had started to refactor it in parallel, you would probably sleep sweet now. But nobody listens to the parents.

~Why bother?

The change is the only one that is constant. Everyone faces the changes every other day. A change of weather or a change requested by the CLIENT. If it is the latter one and you should rebuild the entire app then it is a ringing bell to rewrite your entire code. Impossibility to change and adapt your piece of development work smoothly will decrease the ability of the application to be enhanced with the protection mechanism as they are in an instant updating mode.

Your code is working and you have no clue why. Your code is not working and you have no clue why. And this is a huge smell. It is whispering to you that the debugger will be desperate to process your part of code. Wrong definition and interpretation of the problem will result in unstable assembly, undesirable performance of your programme and put your sensitive data under the risk of breach which is the worst.

To argue about the clean code patterns is the same as discussing politics. And only some bald man will show his shitty code source to face the criticism and jump to the level up. You can think about it the same way as the bloggers on Instagram showing off their life. This is truly a public Code Review! And only hating comments make them stronger. But any developer starts with writing spaghetti code in PHP. Uncle Bob has written millions of dirty coding lines in order to grasp the art of the clear pattern. Please accept that it is okay to write the shitcode. It’s not okay to let it smell.

After conducting the Q&A with the developers from different origins of the programming languages an essential milestone has been revealed: the clean code does not have any smell. Because it does not exist in real life. The end of the story. It behaves like an exhibit in a museum. You can look at but never touch it. The clean code is the ideal one in other words unattainable. But again it depends what a developer puts in its definition. For some it is the elegance in simplicity, for others, it is versatility of functionality. Personally, it implements the concept of security and resilience of an application. The bottom line here is to write in such a clear way so your co-workers would be able to spot the slightly smelling places. It is rather logical that in order to get a clean code you start with writing the dirty one. So it is time to bear in mind while you are discussing in despair what clean patterns are worth, others are resolving the comments from another Code Review and becoming a step closer to the ideal model has been developed in your team specifically.

Application security goes along with the definition of clean code. Everybody will provide a different answer to what security means actually for them. It gets worse with the clients. They simply do not care about it. The term does not exist for them. Likewise with the security having no scent which makes it difficult to nose out.

Well, let’s figure out how to write the code in order to be mega sensitive and as perfume makers be capable of recognizing the preconditions of the issue which is about to stink. According to a variety of whiffs from the strong to the hardly subtle ones we need to comprehend that barely recognised smell would be more likely behind the scented code. And that is the most dangerous.

Important to know:

1. “Good smells” are clearly and easily leading to the core of the issue. And “Bad smells” lie and hide from you.
2. It is slightly possible to write a clean code for mid and junior developers, but it is extremely recommended to write code with “good smells”.
3. The more times you face and struggle in dirt, the more shades of mud you will recognize in future.

~We will go through some of the Smells:

1. Hardcoded URLs
2. URL sanitization in React
3. JSON.Parse
4. Inappropriate Intimacy
5. God Component
6. Weak-types props
7. Not annotated parameters
8. A Lack of comments
9. Over Optimized code
10. Memory Leak

According to Philippe’s article, the most popular attacks are XSS and SQL injection (De Ryck, 2020).

The Smell: Hardcoded URLs

The Problem:

Having any hardcoded values potentially can be a place where malicious code could be injected. In particular characters as <and> is an extremely vulnerable place and the first step to give an attacker a possibility to paste its harmful data which will be interpreted as the valid code of your web application.

The Solution:

Encode your code. I know it sounds weird, but the bottom line is to disguise potentially dangerous characters into harmless symbols seen as data by the browser. Apply context-sensitive pattern output encoding pattern that will transform the characters into the data not confusable for code.

• Never hardcode the credentials of the app.
• Do not use alerts with the hardcoded messages.
• Never assign any value manually to innerHTML! Use dangerouslySetInnerHTML instead
• Avoid hardcoding the URLs, as it contains `/` characters and it leads to the trailing content by comments from the intruders.

The Smell: URL sanitization in React

The Problem:

React is pretty smart to use regex expressions for looking for inappropriate patterns. However, it is able to distinguish only the endpoints beginning with ‘javascript’ and those that start with ‘data’ are doomed.

The Solution:

Do not use the URLs starting with data. Well, quite fun but there is no security-related advice yet. Or use another framework which handles such cases by default.

The Smell: JSON.Parse

The Problem:

dangerouslySetInnerHTML may not work in our favour in case of a more advanced scenario. If the JSON object is being parsed any black hat hacker will dream of making use of this dangerouslySetInnerHTML property. With pleasure, they will embed some infected script configuration into your JSON.

The Solution:

Sanitize the Object before using it! Remember that React will throw the error if the API call createElement contains more than two arguments, so the attack will be prevented. But it will fail if the element has got children.

The Smell: Inappropriate Intimacy

The Problem:

A component having a lot of responsibilities and more than 100 lines is yelling that it will rot soon and turn into a monster. Once you would want to implement a new feature connected with Auth, for instance, you will need to add an exception handler which can create more mess and nobody would be able to recognise where the drawback is hidden due to the length of the logic implementation. Once you come up with the temptation to add the comment explaining some block of code. Attention! It smells!

The Solution:

Take a pencil and start drawing. I am not kidding. It is time to separate the View from the Business logic or refactor some parts into smaller components. And time spending on the sketch will save your time and client’s money in future. Think of React as a tower made of bricks. The components are your bricks tending to have the same size. And only self-discipline multiplied by patience will build the solid defence of your tower.

The Smell: God Component

The Problem:

It is an enormous component with thousands of lines that do extremely more things that it should. It invokes tight coupling, decreases repairability of code.

The Solution:

The component should be divided into the ‘baby’ components with its own business logic and view and execute the context-related assortment of things. Go for the SRP: one component = one responsibility = one part of functionality presented by an application.

The Smell: Weak-types props

The Problem:

Although the powerful React takes care of the prop protection, you as a developer can provide an invalid value to it and create a big scent attracting intruders.

The Solution:

Use Typescript. Encourage your team to wrap a head in and make use of it in order to avoid the situation when an attacker can gain control over your object. If the attacker tries to assign malicious value to your prop, React will reject rendering, but if you do the same, it will end up that the directives will bind an invalid value. It will consequently give an opportunity to the attacker to take advantage of it.

The Smell: Not annotated parameters

The Problem:

Incorrect annotation of variables and parameters can make your programme vulnerable and under threat. The value initially not being assigned might be null or not, might be required or not presenting the true value by default (if hasn’t been overridden).

The Solution:

The parameters should be properly annotated when defining the data models in particular. Along with typing, it is important to mark if the variable can be nulled and give the initial value if overridden. A simple way is to use — serializing the JSON API response. There is no chance you can count on the data from another server. If any property is missing, the null or default values should be handled and processed properly by the front side in order to avoid unexpected results or even failure.

The Smell: A Lack of comments

The Problem:

Yes, it is not a joke. There is a fine line between absence of comments and unnecessary comments. The code itself has no blocks of documenting, no possible view of usage of functions, no explanations where it is needed OR it is full of different comments even with silly context making the code ugly.

The Solution:

A good code is self-documenting. Well, it does not matter that developers should never do commenting at all. The programme is considered to be well-developed if there is no need for commenting as the code tells the genuine plot, but there could be such need for commenting making others to understand why the code fragment exists rather than what it does.

There are two types of comments:

• Comments for documentation: for those who are going to use your code, but never read it. If you develop any platform or library, you will need to establish documentation for your API. Unless you have to update the code you can leave the documentation untouched. But out of the reason there is highly laborious work required, the actual solution is to embed the documentation in the code and with the help of external tools such as JavaDoc to extract it.
• Comments for explanation: for those who are going to read your code. And here it starts to be smelly. Do not overplay with explanations — a bad pattern.

This is the right time for compromising to be mentioned here. Any experienced developer has been familiar with that situation when they go for a bunch of them. App support and speed are the most common and important. As a result, many projects cannot be assessed for the value of the code or full effective usage of any functionality. And what does the average project lead to?

Code reviews bring more and more compromises. The efforts to make the code prettier and nicer are being taken unless developers have enough enthusiasm to enhance the quality going deeper in the colleagues’ code. However, the compromises are getting to turn into the ‘coding dept’ and newly developed parts of the app go to the production stage missing the appropriate refactoring station — ‘don’t touch it if it works’. And all that is being implemented in thousands of TODOs across the app. This is the reason why you can barely find a large production project in the open-source. There are places where explanatory comments have been used, otherwise, you and your team would not guess why that code is doing here. However, those situations should occur extremely seldom.

The Smell: Over Optimized code

The Problem: The optimization costs more than not having the optimization. Or custom hooks are wrapped with useCallback and useMemo every single time while in reality makes the performance slower.

The Solution: The optimization of your code will undesirably increase the complexity of your application, affect the meeting the deadlines and decrease the quality of service. If your code is flexible enough you can continue to optimize it in future if there are such needs. For instance, you are requested to present the project for the company being used by 100 users no later than in one month. And you being crazy about optimizing start building the carcass of the project that will allow one million people to use it. Of course, after the month your project will barely be able to serve 50 users and now you will need even more time to get rid of the old code and start a new one. This is just a waste of time, resources and money.

The Smell: Memory Leak

The Problem: Data breach resources occur when the programme does not allocate the memory properly. It does not empty the useless memory. Cyclic dependencies are the main reason for that smell. There are others as well such as use of React local state as a storage for the project data and calling async tasks without clearing them up.

The Solution: Use Redux to avoid holding data in the local state, it is highly insufficient. Keep an eye over the React hooks implementation, the methods with subscription-like setTimeout must be cleared correctly. The best place for it is useEffect hook which mirrors the functionality of componentDidMount and componentWillUnmount. And for all the XHR requests you have a fancy AbortController to cancel all of them. And even if the AbortController is not helpful then useStateIfMounted should sort it out so your component will render the state once being mounted.

~Conclusive caveats:

Every developer from now and then has experienced the following (3 a.m., the app is half-broken, the console is red and the deadline to submit/present the operational functionality is in the morning). Desperate… Exactly here the security comes to play. Please answer the question: how many times have you come back to the app that has started to work after several sleepless nights and tried to modify the code for the sake of the security? Be honest at least to yourself.

1. While React is pretty safe by design with returning JSX where malicious code cannot be embedded, you can make this impact tended to zero by writing the code which hides the smell. React does have some XSS protection, but a lot of it is still in the hands of the developers.
2. Security: mixes at least two levels of abstraction
3. if you cannot control it, you cannot trust it. Obviously, there is a strong correlation between the clean code and security. But the question is where is that pitfall limit when applying the logic of validation to another level of complexity is being added and it leads to creating the hot desirable place for hackers to come? Complexity hides the bugs not only from your eye but also from the team members reviewing your code.
4. Everyone is yelling about security but when it comes to making things work it goes to the backstage and its concepts become vague… And it’s understandable, as long as the program’s workability is quite obvious to define, the same unfortunately doesn’t come along with the security. Only being hacked would be an appropriate mark evaluating how much your website is/was safe.
5. The same way, everyone is yelling about the clean code, but when it comes to the end of the sprint, it smells `TODO: Improve this later`
6. A clean code is not about the code presented just by one line and you being enormously proud of your graceful developed methods. It basically means the code is expected to be duplicated and some of its parts should be left on the level of abstraction. Theoretically, simplifying your programme is supposed to result in decreasing the amount of defects.
7. Martin, R., n.d. Clean Code. Kindle.
8. Habr.com. 2020. Вероятно, Хватит Рекомендовать “Чистый Код”. [online] Available at: <https://habr.com/ru/post/508876/> [Accessed 19 October 2020].
9. Refactoring.guru. 2020. Code Smells. [online] Available at: <https://refactoring.guru/refactoring/smells> [Accessed 19 October 2020].
10. De Ryck, P., 2020. Preventing XSS In React (Part 1): Data Binding And URLs. [online] Pragmaticwebsecurity.com. Available at: <https://pragmaticwebsecurity.com/articles/spasecurity/react-xss-part1.html> [Accessed 19 October 2020].
11. Hackernoon.com. 2020. Lessons Learned: Common React Code-Smells And How To Avoid Them | Hacker Noon. [online] Available at: <https://hackernoon.com/lessons-learned-common-react-code-smells-and-how-to-avoid-them-f253eb9696a4> [Accessed 19 October 2020].
12. Blackhat.com. 2020. [online] Available at: <https://www.blackhat.com/docs/us-14/materials/us-14-Johns-Call-To-Arms-A-Tale-Of-The-Weaknesses-Of-Current-Client-Side-XSS-Filtering-WP.pdf> [Accessed 19 October 2020].

Originally published at https://kristinavolk.medium.com on December 13, 2020.