Closed
Conversation
Walkthrough이 PR은 애플리케이션에 암호화 인프라를 추가합니다. AES 및 SHA 기반 암호화/해싱 기능을 제공하는 새로운 유틸리티 컴포넌트, 인터페이스, 구현 클래스를 도입하고 환경 변수에서 AES 키를 로드하도록 구성합니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@src/main/java/com/offnal/shifterz/global/util/encrypt/EncryptUtil.java`:
- Around line 19-31: EncryptUtil currently instantiates
SHAEncryptor(SHAType.SHA256/SHA512) which should not be used for password
hashing; replace password-hashing usages with a KDF (e.g., Spring Security
BCryptPasswordEncoder) by adding a BCryptPasswordEncoder bean or field and
updating any callers that use OneWayEncryptor/SHAEncryptor for passwords to use
BCryptPasswordEncoder.encode()/matches() instead; keep SHAEncryptor only for
non-password integrity checks and update OneWayEncryptor Javadoc to clearly
state it is not for password storage and reference SHAEncryptor and SHAType only
for verification/integrity purposes.
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESEncryptor.java`:
- Around line 105-119: The code currently uses platform-default charset in
AESEncryptor.encrypt and in places that recreate strings from bytes (e.g., the
decrypt path and encryptFileToBase64 flow), causing platform-dependent
corruption; update AESEncryptor.encrypt(String plainText) to call
plainText.getBytes(StandardCharsets.UTF_8) (or another explicit charset your app
standard uses) and update any new String(decrypted) usages in the
AESEncryptor.decrypt/related methods to new String(decrypted,
StandardCharsets.UTF_8); for the file/binary path (encryptFileToBase64) ensure
the same explicit charset is used (or better, use Base64 to convert binary to
text) so all conversions are deterministic across platforms.
- Around line 225-240: Validate the input in AESEncryptor.decrypt(byte[]) before
copying: check for null and ensure encryptedArray.length > BLOCK_SIZE (not just
>=) to avoid ArrayIndexOutOfBoundsException and NegativeArraySizeException when
calling System.arraycopy and creating the encrypted byte[]; if validation fails,
throw an IllegalArgumentException with a clear message that includes the
provided length and BLOCK_SIZE to aid debugging (replace the generic
RuntimeException path or perform the check at the top of decrypt to fail fast).
- Around line 42-45: The AESEncryptor constructor currently derives the AES key
via new SHAEncryptor(0).encrypt(key) which is insecure and lacks null/length
validation; replace this with a proper KDF (e.g., PBKDF2WithHmacSHA256 or HKDF)
in the constructor before calling generateSecretKeySpec, using a securely
generated salt and adequate iterations, and ensure you validate the incoming key
parameter (null check and enforce expected byte length) and throw
IllegalArgumentException when the key is null or not the required size as
declared in the Javadoc; update any related helpers so generateSecretKeySpec
consumes the securely-derived key bytes rather than a raw SHA hash.
- Around line 193-213: Current encryptFileToBase64 corrupts binary files by
converting bytes to a String (ISO_8859_1) then calling encryptToBase64; change
the flow to operate on byte[] directly: read File into a byte[] and call a
byte[]-based encryption method instead of string-based encryptToBase64 (add a
byte[] overload to TwoWayEncryptor or a private byte[] encryptBytes -> returns
base64), and update the complementary decryptFileFromBase64 to decode base64 to
byte[] and write bytes to disk; ensure AESEncryptor.encryptFileToBase64 and
AESEncryptor.decryptFileFromBase64 use the new byte[] methods so no
charset-based String conversion occurs.
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAEncryptor.java`:
- Around line 34-37: SHAEncryptor 생성자에서 saltLength 유효성 검증이 빠져 있어 0 이하 값이 들어오면
salt 생성 또는 matches() 동작에 문제가 생깁니다; SHAEncryptor(SHAType shaType, int saltLength)
내부에서 saltLength가 1 이상인지 검증하고 유효하지 않으면 IllegalArgumentException을 던지도록 추가하고(또는
기본값으로 정상 범위 값으로 조정) 이 제약을 클래스 javadoc/생성자 설명에 명시하며, salt를 생성하거나 matches()를 호출하는
내부 메서드(예: salt 생성 로직, matches())는 이 불변 조건을 전제로 동작하도록 유지하세요.
- Around line 12-19: The code exposes and documents MD5 support which is
insecure; update SHAType and SHAEncryptor to remove or deprecate MD5: remove the
MD5 enum member from SHAType (or mark it `@Deprecated` and add a clear deprecation
javadoc), update SHAEncryptor (class-level javadoc and any logic that branches
on SHAType) to no longer advertise or allow MD5 usage at runtime (add defensive
checks that throw/ refuse MD5 if still present), and add a brief javadoc note on
SHAEncryptor recommending secure algorithms (e.g., SHA-256/SHA-512) and
explaining MD5 is deprecated if you choose to keep the enum for backward
compatibility.
🧹 Nitpick comments (8)
build.gradle (1)
68-70: 중복 의존성이 존재합니다.
spring-boot-starter-security(Line 28, 44)와spring-boot-starter-actuator(Line 45, 67)가 각각 두 번 선언되어 있습니다. 빌드 오류는 아니지만 혼동을 줄 수 있으므로 정리가 필요합니다.또한, 이 프로젝트는 이미 Spring Boot 3.5 + JDK 17을 사용하고 있어
java.util.Base64와java.util.HexFormat(JDK 17+)을 사용할 수 있습니다.commons-codec이Hex인코딩 용도로만 필요하다면 JDK 내장 API로 대체 가능한지 검토해 보세요.src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESEncryptor.java (1)
26-28: 상수 필드를private static final로 선언하세요.
BLOCK_SIZE와FILE_BUFFER_SIZE는 인스턴스별로 달라지지 않는 상수이므로static으로 선언하는 것이 적절합니다.♻️ 수정 제안
- private final int BLOCK_SIZE = 16; - private final int FILE_BUFFER_SIZE = 8192; + private static final int BLOCK_SIZE = 16; + private static final int FILE_BUFFER_SIZE = 8192;src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAType.java (1)
5-18:SHATypeenum에 MD5가 포함되어 있어 혼동을 줄 수 있습니다.MD5는 SHA 계열 알고리즘이 아니며, 충돌 공격에 취약하여 보안 목적으로 사용이 권장되지 않습니다. 새로운 암호화 유틸리티에서 MD5를 제공하는 것은 잘못된 사용을 유도할 수 있습니다. 제거를 권장하며, 꼭 필요하다면 enum 이름을
HashType으로 변경하세요.또한,
getMessageDigest()의throws Exception은 범위가 너무 넓습니다.throws NoSuchAlgorithmException으로 구체화하세요.♻️ 수정 제안
+import java.security.NoSuchAlgorithmException; import java.security.MessageDigest; public enum SHAType { SHA256("SHA-256"), - SHA512("SHA-512"), - MD5("MD5"); + SHA512("SHA-512"); public final String algorithm; SHAType(String algorithm) { this.algorithm = algorithm; } - public MessageDigest getMessageDigest() throws Exception { + public MessageDigest getMessageDigest() throws NoSuchAlgorithmException { return MessageDigest.getInstance(algorithm); } }src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESType.java (1)
15-17:getCipher()의throws Exception을 구체화하세요.
Cipher.getInstance()는NoSuchAlgorithmException과NoSuchPaddingException을 던집니다.SHAType과 동일하게 예외 타입을 명시하면 호출자가 적절한 에러 핸들링을 할 수 있습니다.♻️ 수정 제안
+import java.security.NoSuchAlgorithmException; +import javax.crypto.NoSuchPaddingException; + - public Cipher getCipher() throws Exception { + public Cipher getCipher() throws NoSuchAlgorithmException, NoSuchPaddingException { return Cipher.getInstance(transformation); }src/main/java/com/offnal/shifterz/global/util/encrypt/EncryptUtil.java (2)
26-31:this.사용이 일관적이지 않습니다.Line 27-28에서는
this.를 생략하고, Line 29-30에서는this.를 사용하고 있습니다. 일관성을 위해 통일해 주세요.또한, 필드 타입을 구체 클래스(
AESEncryptor,SHAEncryptor) 대신 인터페이스(TwoWayEncryptor,OneWayEncryptor)로 선언하면 추상화에 의존하는 설계 원칙(DIP)에 부합합니다. 단, AES 쪽은encryptFile/decryptFile등 인터페이스에 없는 메서드를 사용하므로, 인터페이스 확장이 필요할 수 있습니다.♻️ `this.` 일관성 수정 제안
private EncryptUtil(`@Value`("${encrypt.aes.key}") String aesKey) { - aesCbcEncryptor = new AESEncryptor(aesKey, AESType.CBC); - aesCtrEncryptor = new AESEncryptor(aesKey, AESType.CTR); + this.aesCbcEncryptor = new AESEncryptor(aesKey, AESType.CBC); + this.aesCtrEncryptor = new AESEncryptor(aesKey, AESType.CTR); this.sha256Encryptor = new SHAEncryptor(SHAType.SHA256); this.sha512Encryptor = new SHAEncryptor(SHAType.SHA512); }
41-53: null 입력에 대한 방어 코드가 없습니다.
plainText나base64EncodedText가null인 경우 하위 encryptor에서NullPointerException이 발생합니다. facade 역할인 이 클래스에서 입력 검증을 수행하면, 더 명확한 에러 메시지를 제공할 수 있습니다.🛡️ 입력 검증 예시
// 각 public 메서드 시작 부분에 추가 Objects.requireNonNull(plainText, "plainText must not be null");src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAEncryptor.java (2)
177-182:SecureRandom을 매 호출마다 생성하는 것은 비효율적입니다.
SecureRandom은 thread-safe하므로 인스턴스 필드나static필드로 재사용하는 것이 좋습니다. 매 호출마다 새로 생성하면 불필요한 시드 초기화 비용이 발생합니다.또한, Javadoc에 "16바이트"로 고정 서술되어 있지만 실제 길이는
saltLength필드에 따라 다릅니다.♻️ SecureRandom 재사용 및 Javadoc 수정 제안
public class SHAEncryptor implements OneWayEncryptor { private static final int DEFAULT_SALT_LENGTH = 16; + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); `@Getter` private final SHAType shaType;/** - * 랜덤 Salt를 생성합니다. - * - * `@return` 생성된 Salt 바이트 배열 (16바이트) + * 랜덤 Salt를 생성합니다. + * + * `@return` 생성된 Salt 바이트 배열 (saltLength 바이트) */ private byte[] generateSalt() { byte[] salt = new byte[saltLength]; - SecureRandom random = new SecureRandom(); - random.nextBytes(salt); + SECURE_RANDOM.nextBytes(salt); return salt; }
162-170: 검증 메서드에서 잘못된 hex 입력 시RuntimeException을 던지는 것은 부적절할 수 있습니다.
matches()는 "일치 여부"를 반환하는 검증 메서드입니다. 잘못된 hex 문자열이 들어오면 일치하지 않는 것으로 간주(return false)하는 것이 호출자 입장에서 더 자연스러울 수 있습니다. 현재 구현은 유효하지 않은 입력에 대해 unchecked exception을 던져 호출자가 try-catch를 강제로 해야 합니다.♻️ 잘못된 입력 시 false 반환 제안
`@Override` public boolean matches(String plainText, String hashedText) { try { byte[] hashedBytes = Hex.decodeHex(hashedText); return matches(plainText, hashedBytes); } catch (Exception e) { - throw new RuntimeException("16진수 해시 디코딩 중 오류 발생", e); + return false; } }
Comment on lines
+19
to
+31
| @Component | ||
| public class EncryptUtil { | ||
| private final AESEncryptor aesCbcEncryptor; | ||
| private final AESEncryptor aesCtrEncryptor; | ||
| private final SHAEncryptor sha256Encryptor; | ||
| private final SHAEncryptor sha512Encryptor; | ||
|
|
||
| private EncryptUtil(@Value("${encrypt.aes.key}") String aesKey) { | ||
| aesCbcEncryptor = new AESEncryptor(aesKey, AESType.CBC); | ||
| aesCtrEncryptor = new AESEncryptor(aesKey, AESType.CTR); | ||
| this.sha256Encryptor = new SHAEncryptor(SHAType.SHA256); | ||
| this.sha512Encryptor = new SHAEncryptor(SHAType.SHA512); | ||
| } |
There was a problem hiding this comment.
비밀번호 해싱에 SHA를 사용하는 것은 보안상 부적절합니다.
OneWayEncryptor Javadoc에 "비밀번호 저장"이 주요 용도로 명시되어 있지만, salted SHA-256/SHA-512는 비밀번호 해싱에 권장되지 않습니다. SHA는 속도가 빠른 해시 함수이므로 brute-force 및 GPU 기반 공격에 취약합니다.
비밀번호 저장에는 bcrypt, scrypt, 또는 Argon2와 같은 key derivation function(KDF)을 사용해야 합니다. Spring Security의 BCryptPasswordEncoder가 이미 이 목적으로 제공됩니다.
현재 SHA 유틸은 데이터 무결성 검증 용도로 한정하고, 비밀번호 해싱은 별도의 KDF 기반 구현을 추가하는 것을 권장합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/offnal/shifterz/global/util/encrypt/EncryptUtil.java`
around lines 19 - 31, EncryptUtil currently instantiates
SHAEncryptor(SHAType.SHA256/SHA512) which should not be used for password
hashing; replace password-hashing usages with a KDF (e.g., Spring Security
BCryptPasswordEncoder) by adding a BCryptPasswordEncoder bean or field and
updating any callers that use OneWayEncryptor/SHAEncryptor for passwords to use
BCryptPasswordEncoder.encode()/matches() instead; keep SHAEncryptor only for
non-password integrity checks and update OneWayEncryptor Javadoc to clearly
state it is not for password storage and reference SHAEncryptor and SHAType only
for verification/integrity purposes.
Comment on lines
+42
to
+45
| public AESEncryptor(String key, AESType aesType) { | ||
| this.aesType = aesType; | ||
| secretKey = generateSecretKeySpec(new SHAEncryptor(0).encrypt(key)); | ||
| } |
There was a problem hiding this comment.
보안 취약: SHA-256 해시를 키 유도 함수(KDF)로 사용하고 있습니다.
new SHAEncryptor(0).encrypt(key)는 단순 SHA-256 해시를 키로 사용합니다. 이는 brute-force 및 dictionary attack에 매우 취약합니다. AES 키 유도에는 PBKDF2WithHmacSHA256 또는 HKDF를 사용해야 합니다.
또한 Javadoc에 @throws IllegalArgumentException 키가 null이거나 32바이트가 아닌 경우라고 명시되어 있지만, 실제로 key에 대한 null 체크나 길이 검증이 없습니다.
🔒 PBKDF2 기반 키 유도 제안
-import com.offnal.shifterz.global.util.encrypt.impl.sha.SHAEncryptor;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
public AESEncryptor(String key, AESType aesType) {
+ if (key == null || key.isEmpty()) {
+ throw new IllegalArgumentException("AES 암호화 키는 null이거나 빈 문자열일 수 없습니다.");
+ }
this.aesType = aesType;
- secretKey = generateSecretKeySpec(new SHAEncryptor(0).encrypt(key));
+ secretKey = deriveKey(key);
+}
+
+private SecretKeySpec deriveKey(String key) {
+ try {
+ // 고정 salt 또는 설정 기반 salt 사용 (기존 암호문과의 호환성 유지 필요)
+ byte[] salt = "offnal-fixed-salt".getBytes(StandardCharsets.UTF_8);
+ PBEKeySpec spec = new PBEKeySpec(key.toCharArray(), salt, 65536, 256);
+ SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
+ byte[] derivedKey = factory.generateSecret(spec).getEncoded();
+ return new SecretKeySpec(derivedKey, "AES");
+ } catch (Exception e) {
+ throw new RuntimeException("AES 키 유도 중 오류 발생", e);
+ }
}🤖 Prompt for AI Agents
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESEncryptor.java`
around lines 42 - 45, The AESEncryptor constructor currently derives the AES key
via new SHAEncryptor(0).encrypt(key) which is insecure and lacks null/length
validation; replace this with a proper KDF (e.g., PBKDF2WithHmacSHA256 or HKDF)
in the constructor before calling generateSecretKeySpec, using a securely
generated salt and adequate iterations, and ensure you validate the incoming key
parameter (null check and enforce expected byte length) and throw
IllegalArgumentException when the key is null or not the required size as
declared in the Javadoc; update any related helpers so generateSecretKeySpec
consumes the securely-derived key bytes rather than a raw SHA hash.
Comment on lines
+105
to
+119
| public byte[] encrypt(String plainText) { | ||
| try { | ||
| Cipher eCipher = aesType.getCipher(); | ||
| byte[] iv = generateIV(); | ||
| initCipher(Cipher.ENCRYPT_MODE, iv, eCipher); | ||
|
|
||
| byte[] encrypted = eCipher.doFinal(plainText.getBytes()); | ||
| byte[] encryptedWithIV = new byte[BLOCK_SIZE + encrypted.length]; | ||
| System.arraycopy(iv, 0, encryptedWithIV, 0, BLOCK_SIZE); | ||
| System.arraycopy(encrypted, 0, encryptedWithIV, BLOCK_SIZE, encrypted.length); | ||
| return encryptedWithIV; | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("AES 암호화 중 오류 발생", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
getBytes()와 new String()에 명시적 charset이 없어 플랫폼 의존적입니다.
Line 111의 plainText.getBytes()와 Line 236의 new String(decrypted)는 플랫폼 기본 charset을 사용합니다. 서버 환경에 따라 암호화/복호화 결과가 달라질 수 있으며, 특히 파일 암호화 경로(encryptFileToBase64 → ISO_8859_1로 생성한 문자열)와의 불일치로 바이너리 데이터가 손상됩니다.
🐛 charset 명시 수정
// Line 111: encrypt 메서드
-byte[] encrypted = eCipher.doFinal(plainText.getBytes());
+byte[] encrypted = eCipher.doFinal(plainText.getBytes(StandardCharsets.UTF_8));
// Line 236: decrypt 메서드
-return new String(decrypted);
+return new String(decrypted, StandardCharsets.UTF_8);Also applies to: 225-240
🤖 Prompt for AI Agents
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESEncryptor.java`
around lines 105 - 119, The code currently uses platform-default charset in
AESEncryptor.encrypt and in places that recreate strings from bytes (e.g., the
decrypt path and encryptFileToBase64 flow), causing platform-dependent
corruption; update AESEncryptor.encrypt(String plainText) to call
plainText.getBytes(StandardCharsets.UTF_8) (or another explicit charset your app
standard uses) and update any new String(decrypted) usages in the
AESEncryptor.decrypt/related methods to new String(decrypted,
StandardCharsets.UTF_8); for the file/binary path (encryptFileToBase64) ensure
the same explicit charset is used (or better, use Base64 to convert binary to
text) so all conversions are deterministic across platforms.
Comment on lines
+193
to
+213
| @Override | ||
| public String encryptFileToBase64(File plainFile) { | ||
| if (!plainFile.exists() || !plainFile.isFile()) { | ||
| throw new IllegalArgumentException("암호화할 파일이 존재하지 않거나 올바른 파일이 아닙니다: " + plainFile.getAbsolutePath()); | ||
| } | ||
|
|
||
| try (FileInputStream fis = new FileInputStream(plainFile); | ||
| ByteArrayOutputStream baos = new ByteArrayOutputStream()) { | ||
|
|
||
| byte[] buffer = new byte[FILE_BUFFER_SIZE]; | ||
| int bytesRead; | ||
| while ((bytesRead = fis.read(buffer)) != -1) { | ||
| baos.write(buffer, 0, bytesRead); | ||
| } | ||
|
|
||
| String fileContentStr = baos.toString(StandardCharsets.ISO_8859_1); | ||
| return encryptToBase64(fileContentStr); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("파일 읽기 중 오류 발생: " + plainFile.getAbsolutePath(), e); | ||
| } | ||
| } |
There was a problem hiding this comment.
바이너리 파일을 String으로 변환하는 방식은 데이터 손상을 유발합니다.
파일 바이트를 ISO_8859_1 String으로 변환 후 encrypt(String) → plainText.getBytes()(기본 charset)를 거치면 round-trip이 깨집니다. 근본적으로 바이너리 데이터를 String 중간 단계 없이 byte[]로 직접 암호화해야 합니다.
🐛 byte[] 기반 암호화 제안
TwoWayEncryptor 인터페이스에 byte[] encrypt(byte[] plainBytes) 오버로드를 추가하거나, 내부적으로 바이트 배열을 직접 처리하는 private 메서드를 만들어 사용하세요:
+private byte[] encryptBytes(byte[] plainBytes) {
+ try {
+ Cipher eCipher = aesType.getCipher();
+ byte[] iv = generateIV();
+ initCipher(Cipher.ENCRYPT_MODE, iv, eCipher);
+
+ byte[] encrypted = eCipher.doFinal(plainBytes);
+ byte[] encryptedWithIV = new byte[BLOCK_SIZE + encrypted.length];
+ System.arraycopy(iv, 0, encryptedWithIV, 0, BLOCK_SIZE);
+ System.arraycopy(encrypted, 0, encryptedWithIV, BLOCK_SIZE, encrypted.length);
+ return encryptedWithIV;
+ } catch (Exception e) {
+ throw new RuntimeException("AES 암호화 중 오류 발생", e);
+ }
+}
+
`@Override`
public String encryptFileToBase64(File plainFile) {
// ... file validation ...
try (FileInputStream fis = new FileInputStream(plainFile);
ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
byte[] buffer = new byte[FILE_BUFFER_SIZE];
int bytesRead;
while ((bytesRead = fis.read(buffer)) != -1) {
baos.write(buffer, 0, bytesRead);
}
- String fileContentStr = baos.toString(StandardCharsets.ISO_8859_1);
- return encryptToBase64(fileContentStr);
+ byte[] encryptedBytes = encryptBytes(baos.toByteArray());
+ return Base64.getEncoder().encodeToString(encryptedBytes);
}
}decryptFileFromBase64도 동일하게 byte[] 기반으로 수정해야 합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESEncryptor.java`
around lines 193 - 213, Current encryptFileToBase64 corrupts binary files by
converting bytes to a String (ISO_8859_1) then calling encryptToBase64; change
the flow to operate on byte[] directly: read File into a byte[] and call a
byte[]-based encryption method instead of string-based encryptToBase64 (add a
byte[] overload to TwoWayEncryptor or a private byte[] encryptBytes -> returns
base64), and update the complementary decryptFileFromBase64 to decode base64 to
byte[] and write bytes to disk; ensure AESEncryptor.encryptFileToBase64 and
AESEncryptor.decryptFileFromBase64 use the new byte[] methods so no
charset-based String conversion occurs.
Comment on lines
+225
to
+240
| @Override | ||
| public String decrypt(byte[] encryptedArray) { | ||
| try { | ||
| Cipher dCipher = aesType.getCipher(); | ||
| byte[] iv = new byte[BLOCK_SIZE]; | ||
| System.arraycopy(encryptedArray, 0, iv, 0, BLOCK_SIZE); | ||
| initCipher(Cipher.DECRYPT_MODE, iv, dCipher); | ||
|
|
||
| byte[] encrypted = new byte[encryptedArray.length - BLOCK_SIZE]; | ||
| System.arraycopy(encryptedArray, BLOCK_SIZE, encrypted, 0, encrypted.length); | ||
| byte[] decrypted = dCipher.doFinal(encrypted); | ||
| return new String(decrypted); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("AES 복호화 중 오류 발생", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
decrypt(byte[]) 메서드에 입력 길이 검증이 없습니다.
encryptedArray가 16바이트(BLOCK_SIZE)보다 작으면 Line 230의 System.arraycopy에서 ArrayIndexOutOfBoundsException이 발생하고, Line 233에서 음수 크기 배열 생성 시도로 NegativeArraySizeException이 발생합니다. 명확한 에러 메시지를 위해 사전 검증이 필요합니다.
🛡️ 입력 검증 추가
`@Override`
public String decrypt(byte[] encryptedArray) {
try {
+ if (encryptedArray == null || encryptedArray.length <= BLOCK_SIZE) {
+ throw new IllegalArgumentException("암호화된 데이터 형식이 올바르지 않습니다. 최소 " + (BLOCK_SIZE + 1) + " 바이트 필요");
+ }
Cipher dCipher = aesType.getCipher();🤖 Prompt for AI Agents
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/aes/AESEncryptor.java`
around lines 225 - 240, Validate the input in AESEncryptor.decrypt(byte[])
before copying: check for null and ensure encryptedArray.length > BLOCK_SIZE
(not just >=) to avoid ArrayIndexOutOfBoundsException and
NegativeArraySizeException when calling System.arraycopy and creating the
encrypted byte[]; if validation fails, throw an IllegalArgumentException with a
clear message that includes the provided length and BLOCK_SIZE to aid debugging
(replace the generic RuntimeException path or perform the check at the top of
decrypt to fail fast).
Comment on lines
+12
to
+19
| /** | ||
| * SHA 해시 암호화 구현 클래스 | ||
| * <p> | ||
| * SHA-256, SHA-512, MD5 알고리즘을 사용한 단방향 해시 암호화를 제공합니다. | ||
| * 모든 해시에 자동으로 Salt를 추가하여 동일한 평문도 다른 해시를 생성합니다. | ||
| * </p> | ||
| */ | ||
| public class SHAEncryptor implements OneWayEncryptor { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find SHAType file
fd "SHAType" --type fRepository: 2025-Shifterz/Offnal-BE
Length of output: 142
🏁 Script executed:
# Also check the full SHAEncryptor.java file to understand the implementation
fd "SHAEncryptor.java" --type f --exec wc -l {}Repository: 2025-Shifterz/Offnal-BE
Length of output: 153
🏁 Script executed:
# Get the directory structure to understand the layout
find src/main/java/com/offnal/shifterz/global/util/encrypt -type f -name "*.java" | head -20Repository: 2025-Shifterz/Offnal-BE
Length of output: 601
🏁 Script executed:
cat -n src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAType.javaRepository: 2025-Shifterz/Offnal-BE
Length of output: 615
🏁 Script executed:
cat -n src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAEncryptor.javaRepository: 2025-Shifterz/Offnal-BE
Length of output: 6082
MD5 알고리즘 지원은 보안상 위험합니다.
SHAType 열거형에 MD5가 정의되어 있고, SHAEncryptor 클래스 문서에 명시적으로 지원된다고 표기되어 있습니다. MD5는 충돌 공격(collision attack)이 실증된 알고리즘으로, 보안 용도에 사용하면 안 됩니다. Salt 추가도 MD5의 근본적인 약점을 해결할 수 없습니다. SHAType에서 MD5 옵션을 제거하거나, 최소한 @Deprecated 처리와 함께 경고 문서를 추가하기를 권장합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAEncryptor.java`
around lines 12 - 19, The code exposes and documents MD5 support which is
insecure; update SHAType and SHAEncryptor to remove or deprecate MD5: remove the
MD5 enum member from SHAType (or mark it `@Deprecated` and add a clear deprecation
javadoc), update SHAEncryptor (class-level javadoc and any logic that branches
on SHAType) to no longer advertise or allow MD5 usage at runtime (add defensive
checks that throw/ refuse MD5 if still present), and add a brief javadoc note on
SHAEncryptor recommending secure algorithms (e.g., SHA-256/SHA-512) and
explaining MD5 is deprecated if you choose to keep the enum for backward
compatibility.
Comment on lines
+34
to
+37
| public SHAEncryptor(SHAType shaType, int saltLength) { | ||
| this.shaType = shaType; | ||
| this.saltLength = saltLength; | ||
| } |
There was a problem hiding this comment.
saltLength에 대한 유효성 검증이 없습니다.
saltLength가 0 이하일 경우 의미 없는 salt가 생성되거나, matches() 로직에서 예기치 않은 동작이 발생할 수 있습니다.
🛡️ 유효성 검증 추가 제안
public SHAEncryptor(SHAType shaType, int saltLength) {
+ if (saltLength <= 0) {
+ throw new IllegalArgumentException("saltLength must be positive");
+ }
this.shaType = shaType;
this.saltLength = saltLength;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public SHAEncryptor(SHAType shaType, int saltLength) { | |
| this.shaType = shaType; | |
| this.saltLength = saltLength; | |
| } | |
| public SHAEncryptor(SHAType shaType, int saltLength) { | |
| if (saltLength <= 0) { | |
| throw new IllegalArgumentException("saltLength must be positive"); | |
| } | |
| this.shaType = shaType; | |
| this.saltLength = saltLength; | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/offnal/shifterz/global/util/encrypt/impl/sha/SHAEncryptor.java`
around lines 34 - 37, SHAEncryptor 생성자에서 saltLength 유효성 검증이 빠져 있어 0 이하 값이 들어오면
salt 생성 또는 matches() 동작에 문제가 생깁니다; SHAEncryptor(SHAType shaType, int saltLength)
내부에서 saltLength가 1 이상인지 검증하고 유효하지 않으면 IllegalArgumentException을 던지도록 추가하고(또는
기본값으로 정상 범위 값으로 조정) 이 제약을 클래스 javadoc/생성자 설명에 명시하며, salt를 생성하거나 matches()를 호출하는
내부 메서드(예: salt 생성 로직, matches())는 이 불변 조건을 전제로 동작하도록 유지하세요.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔀 변경 내용
✅ 작업 항목
📸 스크린샷 (선택)
📎 참고 이슈
OFFNAL-32
Summary by CodeRabbit
릴리스 노트
새로운 기능
의존성