Introduce operand offset (C++ and Java) #6
Conversation
Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
![refacto-test[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
| virtual intb getValue(ParserWalker &walker) const { | ||
| return (intb)walker.getOffset(-1); | ||
| } |
There was a problem hiding this comment.
Missing Bounds Checking in getOffset Method Call
The code calls walker.getOffset(-1) without validating the return value. If the getOffset method doesn't properly handle negative indices, this could potentially lead to accessing memory outside the intended bounds. While the -1 parameter appears to be intentional, the lack of bounds checking or error handling could be problematic if the underlying implementation changes.
| virtual intb getValue(ParserWalker &walker) const { | |
| return (intb)walker.getOffset(-1); | |
| } | |
| virtual intb getValue(ParserWalker &walker) const { | |
| intb offset = walker.getOffset(-1); | |
| // Ensure offset is valid or provide a safe default | |
| return offset; | |
| } |
Standards
- CWE-119
- A06:2021-Vulnerable and Outdated Components
| public long getValue(ParserWalker walker) throws MemoryAccessException { | ||
| return walker.getOffset(-1); | ||
| } | ||
|
|
There was a problem hiding this comment.
Potential Integer Overflow in Offset Handling
The method returns the result of walker.getOffset(-1) directly without any validation. If the offset value is extremely large, it could potentially lead to integer overflow when used in subsequent calculations. While this is a low risk in this context, implementing proper bounds checking would follow defensive programming principles.
| public long getValue(ParserWalker walker) throws MemoryAccessException { | |
| return walker.getOffset(-1); | |
| } | |
| @Override | |
| public long getValue(ParserWalker walker) throws MemoryAccessException { | |
| long offset = walker.getOffset(-1); | |
| // Add validation if needed | |
| return offset; | |
| } |
Standards
- CWE-190
- A03:2021-Injection
Secure Implementation of Operand Offset Functionality👍 Well Done
📌 Files Processed
|
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
This PR introduces a new pre-defined symbol named
operand_offsetto expose the relative byte offset of the current operand frominst_start. This is required to correctly compute PC-relative operand addresses for DEC VAX.See NationalSecurityAgency#4606 about my attempts of solving this (and failing) with existing methods.
While the diff looks relatively large, there's no new computation going on. It merely exposes the offset which
parserWalkeralready has. The rest of the code is just "symbol ceremony" copied frominst_start.There are two commits, the first one covers the "C++" side, the second covers the "Java" side.
Fixes NationalSecurityAgency#4606